[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 at 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