[Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace

[Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace

[Mirian Shilakadze]

mnt->ve_owner is meant to say which VE a mount belongs to. It is read by
ve_check_trusted_file(), which stops ve0 executing content a container
could have written, and by the per-VE mount accounting behind ve.mnt_nr.

It is wrong in two different ways, and each one alone defeats the trusted
exec check. Fixing the second exposed a third bug, a host panic, which is
patch 1 and the reason this is one series rather than two.

Patch 1 fixes the panic. alloc_mnt_ns() links every namespace onto
all_mntns_list and takes a reference on its owning VE, and both are undone
only in free_mnt_ns(). copy_mnt_ns()'s copy_tree() failure path does not
call it, so it frees a namespace that is still linked and leaks the VE
reference, and the next namespace creation runs list_add_tail() through
the dangling entry. A container at its own sysctl_ve_mount_nr limit
calling unshare(CLONE_NEWNS) reaches that path deterministically and
panics the host. This is VSTOR-141545.

Patch 2 fixes ownership not being updated when a mount moves. ve_owner is
assigned once in ve_mount_nr_inc() from alloc_vfsmnt() and never changes,
so a mount the host hands to a container keeps ve_owner == ve0 while
living in the container's mount namespace, and a host tmpfs bindmounted
into a container is trusted even though the container can write to it.
This is VSTOR-141322.

Patch 3 fixes ownership being wrong at creation. A new mount takes its
owner from get_exec_env(), the VE of the task, rather than the VE of the
mount namespace being worked in. Those differ for a ve0 task that entered
a container's mount namespace with nsenter -m. copy_mnt_ns() and
open_detached_copy() both build mounts that way and neither reaches
commit_tree(), so a plain unshare(CLONE_NEWNS) or an
open_tree(OPEN_TREE_CLONE) hands back ve0 owned copies of container
content. This is VSTOR-141429.

Patches 2 and 3 apply the same rule, the one already used when a namespace
is copied at container creation: ownership comes from the mount namespace,
never from the calling task.

Testing
=======

Built on 6.12.0-211.39.1.16.4.vz10, debug flavour, with KASAN, lockdep,
PROVE_LOCKING, DEBUG_ATOMIC_SLEEP, DEBUG_LIST, DEBUG_VM and gcov on
fs/namespace.o. Also compiled with CONFIG_VE=n. Each commit builds
standalone, so bisect is safe.

Before and after on the same host, same commands. Stock
6.12.0-211.30.1.14.4.vz10 on the left, the series on the right:

  container at its mount limit, unshare(CLONE_NEWNS)  PANIC    -> survives
  CT tmpfs, plain nsenter (control)                   refused  -> refused
  nsenter + unshare -m                                EXECUTED -> refused
  nsenter + open_tree + execveat                      EXECUTED -> refused
  host tmpfs lent via --bindmount_add                 EXECUTED -> refused

The panic was captured on stock over netconsole: "list_add corruption ...
kernel BUG at lib/list_debug.c:32", Comm: unshare, ve: 900, trace
alloc_mnt_ns <- copy_mnt_ns <- unshare. Against the series the identical
sequence, 150 container unshares at the limit followed by 300 host
namespace creations, completes with no corruption and no dump. The
control case behaving the same on both kernels shows the difference is
the change and not the environment.

No KASAN, lockdep, atomic sleep, refcount or list corruption reports
across any of it. DEBUG_ATOMIC_SLEEP staying quiet covers the one thing
worth asking about in patch 2, that get_ve()/put_ve() are called under
lock_mount_hash().

ve.mnt_nr drifted by 0 over 15 bindmount add and remove cycles, and
nr_dying_descendants moved by 1 across 150 opportunities to leak a VE
reference, so patch 1 releases the reference rather than merely not
crashing.

Coverage from gcov, so the new code is known to have run rather than just
linked: ve_mount_reown() called 54300 times with the ownership transfer
branch taken 958 times, and both commit_tree() call sites exercised,
20024 for the moved tree and 24081 for the propagation loop.

selftests: mount 2/2, mount_setattr 21/21, ve_perms 14/14, ve_ns_owner
2/2. The last one matters most, it asserts ve.mnt_nr behaviour around
CLONE_NEWVE, which is the line patch 3 changes.

vzctl functional suite: 538 of 603 passed. About 25 of the failures are
vzctl returning exit 21 where the suite asserts 20 for an unrecognized
option, which never reaches the kernel. The other ten were re-run one at
a time: two passed, and the rest fail in container creation or disk setup
on a test filesystem out of space, not on the mount operations they
exercise.

KCSAN, on a separate build of the same series with KCSAN enabled at
runtime: 2520 reports over a 15 minute run, drained continuously so that
is every report rather than what happened to survive in the ring buffer,
while KCSAN's own counter went from 5174 to 10179 data races. None of
them names ve_owner, commit_tree(), ve_check_trusted_file(),
is_sb_ve_accessible() or mnt_ns_unlink(). The code carrying the new store
was hot and instrumented throughout: attach_recursive_mnt(), which calls
commit_tree(), appears in 201 stack traces, and 193 of the reports are
races in propagate_one() and propagate_mnt() beside it. Those are
pre-existing upstream races on inode and mount fields, not on ve_owner.

Not addressed here
==================

Patch 2 lets a container be pushed above sysctl_ve_mount_nr 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, and the container can unmount them, but it is the
host's action that spends the container's budget. Enforcing the limit at
handover is possible, attach_recursive_mnt() already does the equivalent
for sysctl_mount_max in count_mounts(), but ve_mount_allowed() tests the
creating task's VE while the counter follows the owner, so the limit
currently has two meanings and picking one is a separate decision.

Mirian Shilakadze (3):
  ve/fs: unlink the mount namespace on the copy_mnt_ns() error path
  ve/fs: transfer mount ownership when a mount enters another VE
  ve/fs: take the owner of copied mounts from the namespace, not the task

 fs/mount.h     |  2 +-
 fs/namespace.c | 73 ++++++++++++++++++++++++++++++++++++++++++++------
 kernel/ve/ve.c |  6 ++++-
 3 files changed, 71 insertions(+), 10 deletions(-)

-- 
2.43.0

[Devel] [PATCH vz10 1/3] ve/fs: unlink the mount namespace on the copy_mnt_ns() error path

[Mirian Shilakadze]

alloc_mnt_ns() does two things upstream does not: it links the namespace
onto all_mntns_list and takes a reference on its owning VE. Both are
undone in free_mnt_ns(), which holds the only list_del() of mntns_list
and the only put_ve(ns->ve_owner) in the file.

copy_mnt_ns() does not call it when copy_tree() fails. It open codes the
teardown and finishes with mnt_ns_release(), which drops the passive
count and kfree()s the namespace without unlinking it, so the namespace
is freed while all_mntns_list still points at it and the VE reference is
leaked. The next namespace creation runs list_add_tail() through the
dangling entry.

A container reaches this deterministically. alloc_vfsmnt() returns NULL
when !ve_mount_allowed(), that is when the VE is at sysctl_ve_mount_nr,
clone_mnt() turns that into -ENOMEM and copy_tree() propagates it. So a
container sitting at its own mount limit that calls unshare(CLONE_NEWNS)
takes the error path every time, with no memory pressure and nothing
beyond CAP_SYS_ADMIN in its own user namespace, and panics the host:

  list_add corruption. prev->next should be next (ffffffffa9ca77f0), but
  was ff2834a3cdfaeed0. (prev=ff2834a3cdfaeed0).
  kernel BUG at lib/list_debug.c:32!
  CPU: 94 UID: 0 PID: 7139 Comm: unshare ve: 900
   alloc_mnt_ns+0xd5/0x210
   copy_mnt_ns+0x82/0x3c0
   create_new_namespaces+0x5d/0x2f0
   unshare_nsproxy_namespaces+0x69/0xc0
   ksys_unshare+0x213/0x3f0

prev->next == prev is INIT_LIST_HEAD() on reallocated memory, the freed
namespace reused while the list still referenced it. CONFIG_DEBUG_LIST is
only what makes it a clean BUG, without it the same list_add_tail()
writes through the dangling pointer silently.

The path used to call free_mnt_ns() and was correct. Upstream replaced
that with the open coded sequence because free_mnt_ns() reaches
mnt_ns_tree_remove(), which rb_erase()s a node that copy_mnt_ns() has not
inserted yet, mnt_ns_tree_add() running only after the copy succeeds.
Upstream is unaffected by the replacement because its free_mnt_ns()
carries nothing else. Ours does.

So do not restore the free_mnt_ns() call, that would reintroduce the
rb_erase() upstream fixed. Split the part that is ours into
mnt_ns_unlink() and call it from both places, so a future addition to
namespace teardown has one home rather than two that can drift apart,
which is how this happened.

Fixes: 229fd15908fe ("fs: don't try and remove empty rbtree node")
Fixes: 1db60e545f65 ("ve/mntns: add ve_owner to struct mnt_namespace")
https://virtuozzo.atlassian.net/browse/VSTOR-141545
Feature: ve: ve generic structures
Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
 fs/namespace.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4d4dc5290350..7e27537dcdaf 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4243,17 +4243,30 @@ static void dec_mnt_namespaces(struct ucounts *ucounts)
 static LIST_HEAD(all_mntns_list);
 static DEFINE_SPINLOCK(all_mntns_list_lock);
 
-static void free_mnt_ns(struct mnt_namespace *ns)
+/*
+ * Undo the bookkeeping alloc_mnt_ns() sets up beyond what upstream does: the
+ * entry on all_mntns_list and the reference on the owning VE.
+ *
+ * Kept separate from free_mnt_ns() because copy_mnt_ns() has to unwind a
+ * namespace that is not in mnt_ns_tree yet, so it cannot use free_mnt_ns()
+ * without rb_erase()ing a node that was never inserted.
+ */
+static void mnt_ns_unlink(struct mnt_namespace *ns)
 {
-	if (!is_anon_ns(ns))
-		ns_free_inum(&ns->ns);
-	dec_mnt_namespaces(ns->ucounts);
-
 	spin_lock(&all_mntns_list_lock);
 	list_del(&ns->mntns_list);
 	spin_unlock(&all_mntns_list_lock);
 
 	put_ve(ns->ve_owner);
+}
+
+static void free_mnt_ns(struct mnt_namespace *ns)
+{
+	if (!is_anon_ns(ns))
+		ns_free_inum(&ns->ns);
+	dec_mnt_namespaces(ns->ucounts);
+
+	mnt_ns_unlink(ns);
 
 	mnt_ns_tree_remove(ns);
 }
@@ -4347,6 +4360,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 		namespace_unlock();
 		ns_free_inum(&new_ns->ns);
 		dec_mnt_namespaces(new_ns->ucounts);
+		mnt_ns_unlink(new_ns);
 		mnt_ns_release(new_ns);
 		return ERR_CAST(new);
 	}
-- 
2.43.0

[Devel] [PATCH vz10 2/3] ve/fs: transfer mount ownership when a mount enters another VE

[Mirian Shilakadze]

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>
---
 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 5cf06431d586..1d41fd15265e 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 7e27537dcdaf..f8319a2b33df 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 73d1c3b4873e..e976ffcc9273 100644
--- a/kernel/ve/ve.c
+++ b/kernel/ve/ve.c
@@ -1716,9 +1716,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;
 	}
-- 
2.43.0

[Devel] [PATCH vz10 3/3] ve/fs: take the owner of copied mounts from the namespace, not the task

[Mirian Shilakadze]

A new mount takes its ve_owner from get_exec_env(), the VE of the task
doing the work, rather than the VE of the mount namespace it is working
in. The two differ for a ve0 task that has entered a container's mount
namespace and stayed ve0, which is what nsenter -m gives you.

Two places build mounts that way and neither passes through
commit_tree(), so the transfer added by the previous patch cannot correct
them. copy_mnt_ns() takes the owner from the unsharing task unless an
explicit VE was threaded in, which only happens for CLONE_NEWVE, so a
plain unshare(CLONE_NEWNS) hands back a ve0 owned copy of every mount in
the namespace. open_detached_copy() passes NULL to __do_loopback(), so
open_tree(OPEN_TREE_CLONE) mints ve0 owned clones in an anonymous
namespace the mount never leaves before being executed from.

Either one undoes the trusted exec check with one extra command:

  nsenter -t $INITPID -m -- unshare -m -- /ctown/planted

This is older than the mount transfer it defeats. Both paths also launder
a tmpfs the container created entirely on its own, which
ve_check_trusted_file() was already supposed to refuse, so the check has
been avoidable this way since it was added.

Take the owner from the namespace being worked in rather than from the
caller, the same rule commit_tree() follows for a mount that moves. An
explicit VE for a container being created still wins, and nothing changes
for a task working inside its own VE's namespace, which is every normal
mount, bind and unshare.

Note that ve_mount_allowed() tests the limit against get_exec_env() while
ve_mount_nr_inc() charges the owner, so in the mismatched case a ve0 task
passes the check against ve0 and the copies are charged to the container.
That split is older than this patch, which only widens where it applies.

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-141429
Feature: ve: ve generic structures
Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
 fs/namespace.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index f8319a2b33df..4adc1db84b4c 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2830,6 +2830,7 @@ static int do_change_type(struct path *path, int ms_flags)
 
 static struct mount *__do_loopback(struct path *old_path, int recurse)
 {
+	struct ve_struct *owner = current->nsproxy->mnt_ns->ve_owner;
 	struct mount *mnt = ERR_PTR(-EINVAL), *old = real_mount(old_path->mnt);
 
 	if (IS_MNT_UNBINDABLE(old))
@@ -2846,11 +2847,17 @@ static struct mount *__do_loopback(struct path *old_path, int recurse)
 	if (!recurse && __has_locked_children(old, old_path->dentry))
 		return mnt;
 
+	/*
+	 * The copy belongs to the namespace it is taken from, not to whoever
+	 * is asking. The two differ for a ve0 task working inside a
+	 * container's mount namespace, and open_detached_copy() never reaches
+	 * commit_tree() to have the owner corrected later.
+	 */
 	if (recurse)
 		mnt = copy_tree(old, old_path->dentry, CL_COPY_MNT_NS_FILE,
-				NULL);
+				owner);
 	else
-		mnt = clone_mnt(old, old_path->dentry, 0, NULL);
+		mnt = clone_mnt(old, old_path->dentry, 0, owner);
 
 	if (!IS_ERR(mnt))
 		mnt->mnt.mnt_flags &= ~MNT_LOCKED;
@@ -4372,7 +4379,16 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 
 	old = ns->root;
 
-	new_ns = alloc_mnt_ns(user_ns, false, new_ve);
+	/*
+	 * A copied namespace holds copies of @ns's mounts, so it belongs to
+	 * whoever owns @ns rather than to whoever is unsharing. The two differ
+	 * for a ve0 task working inside a container's mount namespace, and
+	 * copy_mnt_ns() populates the namespace directly, without going
+	 * through commit_tree() where the owner would otherwise be corrected.
+	 * An explicit @new_ve still wins: a container being created owns the
+	 * namespace made for it.
+	 */
+	new_ns = alloc_mnt_ns(user_ns, false, new_ve ?: ns->ve_owner);
 	if (IS_ERR(new_ns))
 		return new_ns;
 
-- 
2.43.0

Re: [Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace

[Vasileios Almpanis]

> mnt->ve_owner is meant to say which VE a mount belongs to. It is read by
> ve_check_trusted_file(), which stops ve0 executing content a container
> could have written, and by the per-VE mount accounting behind ve.mnt_nr.
> 
> It is wrong in two different ways, and each one alone defeats the trusted
> exec check. Fixing the second exposed a third bug, a host panic, which is
> patch 1 and the reason this is one series rather than two.
> 
> Patch 1 fixes the panic. alloc_mnt_ns() links every namespace onto
> all_mntns_list and takes a reference on its owning VE, and both are undone
> only in free_mnt_ns(). copy_mnt_ns()'s copy_tree() failure path does not
> call it, so it frees a namespace that is still linked and leaks the VE
> reference, and the next namespace creation runs list_add_tail() through
> the dangling entry. A container at its own sysctl_ve_mount_nr limit
> calling unshare(CLONE_NEWNS) reaches that path deterministically and
> panics the host. This is VSTOR-141545.
> 
> Patch 2 fixes ownership not being updated when a mount moves. ve_owner is
> assigned once in ve_mount_nr_inc() from alloc_vfsmnt() and never changes,
> so a mount the host hands to a container keeps ve_owner == ve0 while
> living in the container's mount namespace, and a host tmpfs bindmounted
> into a container is trusted even though the container can write to it.
> This is VSTOR-141322.
> 
> Patch 3 fixes ownership being wrong at creation. A new mount takes its
> owner from get_exec_env(), the VE of the task, rather than the VE of the
> mount namespace being worked in. Those differ for a ve0 task that entered
> a container's mount namespace with nsenter -m. copy_mnt_ns() and
> open_detached_copy() both build mounts that way and neither reaches
> commit_tree(), so a plain unshare(CLONE_NEWNS) or an
> open_tree(OPEN_TREE_CLONE) hands back ve0 owned copies of container
> content. This is VSTOR-141429.
> 
> Patches 2 and 3 apply the same rule, the one already used when a namespace
> is copied at container creation: ownership comes from the mount namespace,
> never from the calling task.
> 
Generally LGTM. The only problematic scenario I see now that we gate
host execution based on namespace is that we could have tmpfs with two
mounts one in host and one in CT. CT can still tamper with things and on
the fs and ve_check_trusted_file will still return true allowing us to
execute on host. I don't thing this should be covered in this series as its
irrelevant I just wanted to mention it in case other reviewers think we
need to do something about it. If deemed necessary we could solve it
in O(1) time, by using some superblock flag and checking against that
in ve_check_trusted_file.
> Testing
> =======
> 
> Built on 6.12.0-211.39.1.16.4.vz10, debug flavour, with KASAN, lockdep,
> PROVE_LOCKING, DEBUG_ATOMIC_SLEEP, DEBUG_LIST, DEBUG_VM and gcov on
> fs/namespace.o. Also compiled with CONFIG_VE=n. Each commit builds
> standalone, so bisect is safe.
> 
> Before and after on the same host, same commands. Stock
> 6.12.0-211.30.1.14.4.vz10 on the left, the series on the right:
> 
>   container at its mount limit, unshare(CLONE_NEWNS)  PANIC    -> survives
>   CT tmpfs, plain nsenter (control)                   refused  -> refused
>   nsenter + unshare -m                                EXECUTED -> refused
>   nsenter + open_tree + execveat                      EXECUTED -> refused
>   host tmpfs lent via --bindmount_add                 EXECUTED -> refused
> 
> The panic was captured on stock over netconsole: "list_add corruption ...
> kernel BUG at lib/list_debug.c:32", Comm: unshare, ve: 900, trace
> alloc_mnt_ns <- copy_mnt_ns <- unshare. Against the series the identical
> sequence, 150 container unshares at the limit followed by 300 host
> namespace creations, completes with no corruption and no dump. The
> control case behaving the same on both kernels shows the difference is
> the change and not the environment.
> 
> No KASAN, lockdep, atomic sleep, refcount or list corruption reports
> across any of it. DEBUG_ATOMIC_SLEEP staying quiet covers the one thing
> worth asking about in patch 2, that get_ve()/put_ve() are called under
> lock_mount_hash().
> 
> ve.mnt_nr drifted by 0 over 15 bindmount add and remove cycles, and
> nr_dying_descendants moved by 1 across 150 opportunities to leak a VE
> reference, so patch 1 releases the reference rather than merely not
> crashing.
> 
> Coverage from gcov, so the new code is known to have run rather than just
> linked: ve_mount_reown() called 54300 times with the ownership transfer
> branch taken 958 times, and both commit_tree() call sites exercised,
> 20024 for the moved tree and 24081 for the propagation loop.
> 
> selftests: mount 2/2, mount_setattr 21/21, ve_perms 14/14, ve_ns_owner
> 2/2. The last one matters most, it asserts ve.mnt_nr behaviour around
> CLONE_NEWVE, which is the line patch 3 changes.
> 
> vzctl functional suite: 538 of 603 passed. About 25 of the failures are
> vzctl returning exit 21 where the suite asserts 20 for an unrecognized
> option, which never reaches the kernel. The other ten were re-run one at
> a time: two passed, and the rest fail in container creation or disk setup
> on a test filesystem out of space, not on the mount operations they
> exercise.
> 
> KCSAN, on a separate build of the same series with KCSAN enabled at
> runtime: 2520 reports over a 15 minute run, drained continuously so that
> is every report rather than what happened to survive in the ring buffer,
> while KCSAN's own counter went from 5174 to 10179 data races. None of
> them names ve_owner, commit_tree(), ve_check_trusted_file(),
> is_sb_ve_accessible() or mnt_ns_unlink(). The code carrying the new store
> was hot and instrumented throughout: attach_recursive_mnt(), which calls
> commit_tree(), appears in 201 stack traces, and 193 of the reports are
> races in propagate_one() and propagate_mnt() beside it. Those are
> pre-existing upstream races on inode and mount fields, not on ve_owner.
> 
> Not addressed here
> ==================
> 
> Patch 2 lets a container be pushed above sysctl_ve_mount_nr 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, and the container can unmount them, but it is the
> host's action that spends the container's budget. Enforcing the limit at
> handover is possible, attach_recursive_mnt() already does the equivalent
> for sysctl_mount_max in count_mounts(), but ve_mount_allowed() tests the
> creating task's VE while the counter follows the owner, so the limit
> currently has two meanings and picking one is a separate decision.
> 
> Mirian Shilakadze (3):
>   ve/fs: unlink the mount namespace on the copy_mnt_ns() error path
>   ve/fs: transfer mount ownership when a mount enters another VE
>   ve/fs: take the owner of copied mounts from the namespace, not the task
> 
>  fs/mount.h     |  2 +-
>  fs/namespace.c | 73 ++++++++++++++++++++++++++++++++++++++++++++------
>  kernel/ve/ve.c |  6 ++++-
>  3 files changed, 71 insertions(+), 10 deletions(-)
> 
> --
> 2.43.0
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel

Reviewed-by: Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

-- 
Vasileios Almpanis <vasileios.almpanis@virtuozzo.com>

Re: [Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace

[Konstantin Khorenko]

On 8/18/26 12:40, Vasileios Almpanis wrote:

>>
> Generally LGTM. The only problematic scenario I see now that we gate
> host execution based on namespace is that we could have tmpfs with two
> mounts one in host and one in CT. CT can still tamper with things and on
> the fs and ve_check_trusted_file will still return true allowing us to
> execute on host. I don't thing this should be covered in this series as its
> irrelevant I just wanted to mention it in case other reviewers think we
> need to do something about it. If deemed necessary we could solve it
> in O(1) time, by using some superblock flag and checking against that
> in ve_check_trusted_file.

Absolutely agree, so we need to handle this scenario as well,
i have filed a separate bug for that:
https://virtuozzo.atlassian.net/browse/VSTOR-142982


_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RHEL10 COMMIT] ve/fs: unlink the mount namespace on the copy_mnt_ns() error path

[Konstantin Khorenko]

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 5e3e3c3facbcd29dfa49f7f6ab5aabb39a60d1f9
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Mon Aug 17 11:16:39 2026 +0400

    ve/fs: unlink the mount namespace on the copy_mnt_ns() error path
    
    alloc_mnt_ns() does two things upstream does not: it links the namespace
    onto all_mntns_list and takes a reference on its owning VE. Both are
    undone in free_mnt_ns(), which holds the only list_del() of mntns_list
    and the only put_ve(ns->ve_owner) in the file.
    
    copy_mnt_ns() does not call it when copy_tree() fails. It open codes the
    teardown and finishes with mnt_ns_release(), which drops the passive
    count and kfree()s the namespace without unlinking it, so the namespace
    is freed while all_mntns_list still points at it and the VE reference is
    leaked. The next namespace creation runs list_add_tail() through the
    dangling entry.
    
    A container reaches this deterministically. alloc_vfsmnt() returns NULL
    when !ve_mount_allowed(), that is when the VE is at sysctl_ve_mount_nr,
    clone_mnt() turns that into -ENOMEM and copy_tree() propagates it. So a
    container sitting at its own mount limit that calls unshare(CLONE_NEWNS)
    takes the error path every time, with no memory pressure and nothing
    beyond CAP_SYS_ADMIN in its own user namespace, and panics the host:
    
      list_add corruption. prev->next should be next (ffffffffa9ca77f0), but
      was ff2834a3cdfaeed0. (prev=ff2834a3cdfaeed0).
      kernel BUG at lib/list_debug.c:32!
      CPU: 94 UID: 0 PID: 7139 Comm: unshare ve: 900
       alloc_mnt_ns+0xd5/0x210
       copy_mnt_ns+0x82/0x3c0
       create_new_namespaces+0x5d/0x2f0
       unshare_nsproxy_namespaces+0x69/0xc0
       ksys_unshare+0x213/0x3f0
    
    prev->next == prev is INIT_LIST_HEAD() on reallocated memory, the freed
    namespace reused while the list still referenced it. CONFIG_DEBUG_LIST is
    only what makes it a clean BUG, without it the same list_add_tail()
    writes through the dangling pointer silently.
    
    The path used to call free_mnt_ns() and was correct. Upstream replaced
    that with the open coded sequence because free_mnt_ns() reaches
    mnt_ns_tree_remove(), which rb_erase()s a node that copy_mnt_ns() has not
    inserted yet, mnt_ns_tree_add() running only after the copy succeeds.
    Upstream is unaffected by the replacement because its free_mnt_ns()
    carries nothing else. Ours does.
    
    So do not restore the free_mnt_ns() call, that would reintroduce the
    rb_erase() upstream fixed. Split the part that is ours into
    mnt_ns_unlink() and call it from both places, so a future addition to
    namespace teardown has one home rather than two that can drift apart,
    which is how this happened.
    
    Fixes: 229fd15908fe ("fs: don't try and remove empty rbtree node")
    Fixes: 1db60e545f65 ("ve/mntns: add ve_owner to struct mnt_namespace")
    https://virtuozzo.atlassian.net/browse/VSTOR-141545
    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/namespace.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index 4d4dc52903508..7e27537dcdaf9 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -4243,17 +4243,30 @@ static void dec_mnt_namespaces(struct ucounts *ucounts)
 static LIST_HEAD(all_mntns_list);
 static DEFINE_SPINLOCK(all_mntns_list_lock);
 
-static void free_mnt_ns(struct mnt_namespace *ns)
+/*
+ * Undo the bookkeeping alloc_mnt_ns() sets up beyond what upstream does: the
+ * entry on all_mntns_list and the reference on the owning VE.
+ *
+ * Kept separate from free_mnt_ns() because copy_mnt_ns() has to unwind a
+ * namespace that is not in mnt_ns_tree yet, so it cannot use free_mnt_ns()
+ * without rb_erase()ing a node that was never inserted.
+ */
+static void mnt_ns_unlink(struct mnt_namespace *ns)
 {
-	if (!is_anon_ns(ns))
-		ns_free_inum(&ns->ns);
-	dec_mnt_namespaces(ns->ucounts);
-
 	spin_lock(&all_mntns_list_lock);
 	list_del(&ns->mntns_list);
 	spin_unlock(&all_mntns_list_lock);
 
 	put_ve(ns->ve_owner);
+}
+
+static void free_mnt_ns(struct mnt_namespace *ns)
+{
+	if (!is_anon_ns(ns))
+		ns_free_inum(&ns->ns);
+	dec_mnt_namespaces(ns->ucounts);
+
+	mnt_ns_unlink(ns);
 
 	mnt_ns_tree_remove(ns);
 }
@@ -4347,6 +4360,7 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 		namespace_unlock();
 		ns_free_inum(&new_ns->ns);
 		dec_mnt_namespaces(new_ns->ucounts);
+		mnt_ns_unlink(new_ns);
 		mnt_ns_release(new_ns);
 		return ERR_CAST(new);
 	}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Konstantin Khorenko]

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

Re: [Devel] [PATCH RHEL10 COMMIT] ve/fs: take the owner of copied mounts from the namespace, not the task

[Konstantin Khorenko]

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 a614531f0672e6ab3ad4f13c91fe813dc8927009
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Mon Aug 17 11:16:41 2026 +0400

    ve/fs: take the owner of copied mounts from the namespace, not the task
    
    A new mount takes its ve_owner from get_exec_env(), the VE of the task
    doing the work, rather than the VE of the mount namespace it is working
    in. The two differ for a ve0 task that has entered a container's mount
    namespace and stayed ve0, which is what nsenter -m gives you.
    
    Two places build mounts that way and neither passes through
    commit_tree(), so the transfer added by the previous patch cannot correct
    them. copy_mnt_ns() takes the owner from the unsharing task unless an
    explicit VE was threaded in, which only happens for CLONE_NEWVE, so a
    plain unshare(CLONE_NEWNS) hands back a ve0 owned copy of every mount in
    the namespace. open_detached_copy() passes NULL to __do_loopback(), so
    open_tree(OPEN_TREE_CLONE) mints ve0 owned clones in an anonymous
    namespace the mount never leaves before being executed from.
    
    Either one undoes the trusted exec check with one extra command:
    
      nsenter -t $INITPID -m -- unshare -m -- /ctown/planted
    
    This is older than the mount transfer it defeats. Both paths also launder
    a tmpfs the container created entirely on its own, which
    ve_check_trusted_file() was already supposed to refuse, so the check has
    been avoidable this way since it was added.
    
    Take the owner from the namespace being worked in rather than from the
    caller, the same rule commit_tree() follows for a mount that moves. An
    explicit VE for a container being created still wins, and nothing changes
    for a task working inside its own VE's namespace, which is every normal
    mount, bind and unshare.
    
    Note that ve_mount_allowed() tests the limit against get_exec_env() while
    ve_mount_nr_inc() charges the owner, so in the mismatched case a ve0 task
    passes the check against ve0 and the copies are charged to the container.
    That split is older than this patch, which only widens where it applies.
    
    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-141429
    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/namespace.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/fs/namespace.c b/fs/namespace.c
index f8319a2b33df8..4adc1db84b4c0 100644
--- a/fs/namespace.c
+++ b/fs/namespace.c
@@ -2830,6 +2830,7 @@ static int do_change_type(struct path *path, int ms_flags)
 
 static struct mount *__do_loopback(struct path *old_path, int recurse)
 {
+	struct ve_struct *owner = current->nsproxy->mnt_ns->ve_owner;
 	struct mount *mnt = ERR_PTR(-EINVAL), *old = real_mount(old_path->mnt);
 
 	if (IS_MNT_UNBINDABLE(old))
@@ -2846,11 +2847,17 @@ static struct mount *__do_loopback(struct path *old_path, int recurse)
 	if (!recurse && __has_locked_children(old, old_path->dentry))
 		return mnt;
 
+	/*
+	 * The copy belongs to the namespace it is taken from, not to whoever
+	 * is asking. The two differ for a ve0 task working inside a
+	 * container's mount namespace, and open_detached_copy() never reaches
+	 * commit_tree() to have the owner corrected later.
+	 */
 	if (recurse)
 		mnt = copy_tree(old, old_path->dentry, CL_COPY_MNT_NS_FILE,
-				NULL);
+				owner);
 	else
-		mnt = clone_mnt(old, old_path->dentry, 0, NULL);
+		mnt = clone_mnt(old, old_path->dentry, 0, owner);
 
 	if (!IS_ERR(mnt))
 		mnt->mnt.mnt_flags &= ~MNT_LOCKED;
@@ -4372,7 +4379,16 @@ struct mnt_namespace *copy_mnt_ns(unsigned long flags, struct mnt_namespace *ns,
 
 	old = ns->root;
 
-	new_ns = alloc_mnt_ns(user_ns, false, new_ve);
+	/*
+	 * A copied namespace holds copies of @ns's mounts, so it belongs to
+	 * whoever owns @ns rather than to whoever is unsharing. The two differ
+	 * for a ve0 task working inside a container's mount namespace, and
+	 * copy_mnt_ns() populates the namespace directly, without going
+	 * through commit_tree() where the owner would otherwise be corrected.
+	 * An explicit @new_ve still wins: a container being created owns the
+	 * namespace made for it.
+	 */
+	new_ns = alloc_mnt_ns(user_ns, false, new_ve ?: ns->ve_owner);
 	if (IS_ERR(new_ns))
 		return new_ns;
 
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel