[Devel] [PATCH vz10 0/3] ve/fs: make mount ownership follow the mount namespace
Vasileios Almpanis
vasileios.almpanis at virtuozzo.com
Tue Aug 18 13:40:14 MSK 2026
> 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 at virtuozzo.com>
--
Vasileios Almpanis <vasileios.almpanis at virtuozzo.com>
More information about the Devel
mailing list