Re: [Devel] [PATCH RHEL10 COMMIT] ve/fs: transfer mount ownership when a mount enters another VE

[Konstantin Khorenko <khorenko@virtuozzo.com>]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.10.vz10
------>
commit fce5f612701953f243851cc79415c99e2f995ce6
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Mon Aug 17 11:16:40 2026 +0400

    ve/fs: transfer mount ownership when a mount enters another VE
    
    ve_owner is assigned once in ve_mount_nr_inc() from alloc_vfsmnt() and
    nothing updates it afterwards, so a mount the host creates and moves into
    a container keeps ve_owner == ve0 while it lives in the container's mount
    namespace.
    
    ve_check_trusted_file() reads that field for filesystems with no s_bdev
    and lets ve0 execute from any mount it considers host owned, so a host
    tmpfs bindmounted into a container is trusted even though the container
    can write to it. A tmpfs the container creates itself is refused, the
    same binary planted by the same container on a mount the host lent it
    runs:
    
      vzctl set 971 --bindmount_add /root/tex_tmpfs:/mnt/bm_tmpfs --save
      vzctl exec 971 'cp /bin/echo /mnt/bm_tmpfs/planted &&
                      chmod 755 /mnt/bm_tmpfs/planted'
      nsenter -t $INITPID -m /mnt/bm_tmpfs/planted HOST-TMPFS-LENT-TO-CT
      HOST-TMPFS-LENT-TO-CT
    
    Fix it where the mount changes hands. Every path that puts an existing
    mount into another VE's namespace goes through commit_tree(), both the
    detached tree the container moves in with move_mount() and the copies the
    propagation loop in attach_recursive_mnt() commits into a foreign
    namespace, so adopt the namespace's owner there. Ownership describes
    where the mount is, so it follows the namespace and the direction of the
    move is not special cased.
    
    That covers every mount that moves. Mounts created with the wrong owner
    to begin with, which copy_mnt_ns() and open_detached_copy() both do for a
    ve0 task working inside a container, never pass through commit_tree() and
    are fixed by the next patch.
    
    The same field drives per-VE mount accounting, which was wrong in the
    same direction: a moved in mount was charged to ve0 rather than to the
    container holding it.
    
    A ve0 process that execs or mmaps from a mount this reowns is now
    refused, exec with -EACCES and mmap with -EBADF, plus a SIGSEGV for the
    first few attempts. That is the point of the change, but it is visible to
    host tooling that reaches into a container's mounts to run something.
    
    The transfer does not consult sysctl_ve_mount_nr, commit_tree() cannot
    fail. A container can therefore be pushed above its mount limit by mounts
    the host gives it, and while over it the container's own mounts are
    refused until the count drops. The default limit is 4096 so this takes an
    unusual number of lent mounts, but it is the host's action that spends
    the container's budget.
    
    The owner change is done under the vfsmount lock, which serializes it
    against is_sb_ve_accessible() walking sb->s_mounts.
    ve_check_trusted_file() reads ve_owner without that lock, so both the
    store and the load are marked. It only compares the pointer against ve0
    and the field is never NULL in between, so the reader sees either the old
    or the new owner. The reference on the old VE is dropped only after the
    new one is taken.
    
    Fixes: d65efacf542b ("trusted/ve/fs/exec: Don't allow a privileged user to execute untrusted files")
    Fixes: fc7157b84c32 ("trusted/ve/mmap: Protect from unsecure library load from CT image")
    https://virtuozzo.atlassian.net/browse/VSTOR-141322
    Feature: ve: ve generic structures
    Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
    Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/mount.h     |  2 +-
 fs/namespace.c | 27 +++++++++++++++++++++++++++
 kernel/ve/ve.c |  6 +++++-
 3 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/fs/mount.h b/fs/mount.h
index 5cf06431d5868..1d41fd15265e6 100644
--- a/fs/mount.h
+++ b/fs/mount.h
@@ -71,7 +71,7 @@ struct mount {
 	};
 	struct list_head mnt_umounting; /* list entry for umount propagation */
 #ifdef CONFIG_VE
-	struct ve_struct *ve_owner;	/* VE in which this mount was created */
+	struct ve_struct *ve_owner;	/* VE whose mount namespace holds it */
 #endif /* CONFIG_VE */
 #ifdef CONFIG_FSNOTIFY
 	struct fsnotify_mark_connector __rcu *mnt_fsnotify_marks;
diff --git a/fs/namespace.c b/fs/namespace.c
index 7e27537dcdaf9..f8319a2b33df8 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -320,6 +320,7 @@ int mnt_get_count(struct mount *mnt)
 static inline int ve_mount_allowed(void);
 static inline void ve_mount_nr_inc(struct mount *mnt, struct ve_struct *ve);
 static inline void ve_mount_nr_dec(struct mount *mnt);
+static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve);
 
 static struct mount *alloc_vfsmnt(const char *name, struct ve_struct *owner_ve)
 {
@@ -1182,6 +1183,7 @@ static void commit_tree(struct mount *mnt)
 		m = list_first_entry(&head, typeof(*m), mnt_list);
 		list_del(&m->mnt_list);
 
+		ve_mount_reown(m, n->ve_owner);
 		mnt_add_to_ns(n, m);
 	}
 	n->nr_mounts += n->pending_mounts;
@@ -3370,6 +3372,30 @@ static inline void ve_mount_nr_dec(struct mount *mnt)
 	mnt->ve_owner = NULL;
 }
 
+/*
+ * A mount that enters the mount namespace of another VE changes hands, so
+ * that per-VE mount accounting and the trusted-exec check see it as owned by
+ * the VE it now lives in. Ownership describes where the mount is, so it
+ * simply follows the namespace and nothing here special cases which way the
+ * mount travelled.
+ *
+ * vfsmount lock must be held for write, it serializes the owner change
+ * against is_sb_ve_accessible(). The trusted-exec path reads ve_owner
+ * locklessly but only compares the pointer, which is never NULL here.
+ */
+static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve)
+{
+	struct ve_struct *old = mnt->ve_owner;
+
+	if (old == ve)
+		return;
+
+	atomic_inc(&ve->mnt_nr);
+	WRITE_ONCE(mnt->ve_owner, get_ve(ve));
+	atomic_dec(&old->mnt_nr);
+	put_ve(old);
+}
+
 bool is_sb_ve_accessible(struct ve_struct *ve, struct super_block *sb)
 {
 	struct mount *mnt;
@@ -3392,6 +3418,7 @@ bool is_sb_ve_accessible(struct ve_struct *ve, struct super_block *sb)
 static inline int ve_mount_allowed(void) { return 1; }
 static inline void ve_mount_nr_inc(struct mount *mnt, struct ve_struct *ve) { }
 static inline void ve_mount_nr_dec(struct mount *mnt) { }
+static inline void ve_mount_reown(struct mount *mnt, struct ve_struct *ve) { }
 #endif /* CONFIG_VE */
 
 /*
diff --git a/kernel/ve/ve.c b/kernel/ve/ve.c
index 750a1b2882a7d..b2788b80e7655 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -1727,9 +1727,13 @@ static bool ve_check_trusted_file(struct file *file)
 		/*
 		 * bdev can be NULL if the file is on tmpfs, for example.
 		 * If this is a host's tmpfs - execution is allowed.
+		 *
+		 * The read is unlocked and pairs with the store in
+		 * ve_mount_reown(): a task holding a descriptor on the mount
+		 * can be executing from it while the mount changes hands.
 		 */
 		file_on_host_mount = ve_is_super(
-				     real_mount(file->f_path.mnt)->ve_owner);
+			READ_ONCE(real_mount(file->f_path.mnt)->ve_owner));
 		if (file_on_host_mount)
 			return true;
 	}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel