From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mirian Shilakadze Date: Mon, 17 Aug 2026 11:16:38 +0400 Subject: [Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace Message-ID: List-Id: 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