[Devel] [PATCH vz10 0/2] fs/kernfs, ve: stop a container lookup unmounting host filesystems

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

Except for improvement note on the test (to avoid excess mountinfo reading),
looks good.

We've discussed my comment to the first patch in private and both options are ok,
as we don't really need invalidating cause such a dentry which is hidden by
our sysfs rules can only be created non-negative by host, so invalidating it
should not be really needed. So both options (with extra namespace check and
without) are OK. Extra namespace check also covers case when host for some
reason created the dentry VE can't see on VE's superblock of sysfs.

On 8/25/26 15:29, Mirian Shilakadze wrote:
> Starting a container whose configuration carries a bindmount whose source is
> a mount with its own superblock unmounts the host's bpffs and tracefs.
> libvzctl needs bpffs for the cgroup v2 device controller, so once it is gone
> no container on the node can be managed. Every later vzctl command on any
> container, including ones that were already running and were never involved,
> prints "Unable to find mount point for bpf" twice and then reports a stale
> status. Recovery is a manual mount or a reboot. This is VSTOR-142552.
> 
> The container start is not what does it. Any task whose VE is a container's,
> resolving a host path under /sys, unmounts what it finds there. setns() on a
> container's ve namespace, staying in the host mount namespace, is enough, and
> one stat() of /sys/fs/bpf both hides the entry from the caller and destroys
> the host's mount.
> 
> kernfs_dop_revalidate() ends with a per VE visibility check and answers it
> with the same "return 0" that the staleness checks above it use. Those checks
> are properties of the kernfs node and hold for every observer: the node was
> deactivated, moved, renamed, or retagged. Visibility is a property of the
> calling task's VE, so one host dentry answers "valid" to a ve0 task and
> "stale" to a task inside a container. The VFS reads 0 as a global fact and
> calls d_invalidate(), which hands every mountpoint under that dentry to
> __detach_mounts(), whose mountpoint hash is not scoped to a mount namespace
> and whose m_list holds every mount attached at that dentry in any of them. A
> per VE answer therefore destroys a global object.
> 
> Patch 1 returns -ENOENT from that one check. The caller that cannot see the
> entry is told the name is missing, which is what the check is for, and the
> dentry stays valid for everyone else. What a container is told does not
> change: the errno for a hidden entry is ENOENT either way, because today it
> arrives after d_invalidate() and a fresh lookup that ends in a negative
> dentry. kernfs_iop_lookup() has always answered this same condition with a
> plain "not found", and the revalidate site now matches it.
> 
> The one exception is a create attempt on a hidden name inside a container. It
> now fails with ENOENT when the dentry is cached and continues to fail with
> EACCES when it is not. Both fail, and the cold path is not touched by this
> patch.
> 
> Patch 2 adds the regression test to the existing ve_perms selftest. It mounts
> a tmpfs on the entry the fixture already keeps host only, in its own mount
> namespace so the machine running it cannot lose a mount it needs, and
> requires that mount to still be there after a VE has looked the entry up.
> 
> Introduced by 3dd8c2499df6 ("ve/kernfs: hide forbidden entries in container")
> in 2021 and reachable ever since. It went unreported because nothing in the
> management stack held a mount under /sys that anyone would miss, until
> libvzctl commit f946fae ("cgroup: switch from cgrou-v1 device controller to
> eBPF program") made it depend on bpffs.
> 
> Testing
> =======
> 
> Tested on a VHI 8.0.0 node with the same script, the same container and the
> same bindmount on both kernels.
> 
> On stock 6.12.0-211.30.1.14.4.vz10 the start fails with rc=255 and "Cancel
> init execution", bpffs and tracefs are both gone afterwards, vzctl status on
> that container and on an unrelated one prints "Unable to find mount point for
> bpf" twice each, and vzctl exec stops working. Losing tracefs also took the
> kprobes the test itself was using.
> 
> With patch 1 on 6.12.0-211.39.1.16.9.vz10 the same start returns rc=0, bpffs
> and tracefs are untouched, both status calls are clean, and the bindmount is
> present inside the container and read only as requested. The same holds on a
> debug build with KASAN and lockdep and on the shipping configuration.
> 
> Under load, 48 processes inside a container's VE entered with setns(),
> alongside 48 in ve0, resolved /sys/fs/bpf and a tmpfs mounted on a hidden
> sysfs directory, 384000 hidden lookups in total. Every VE process saw ENOENT
> on every lookup and every ve0 process saw the entry on every lookup, with no
> mixed results. A kprobe on d_invalidate() named only the test's own cgroup
> dentries and the /proc pid directories of reaped children, never the hidden
> entries, and __detach_mounts() was never called. gcov on fs/kernfs/dir.c,
> fs/namei.c, fs/dcache.c and fs/namespace.c agrees: the new return ran 384000
> times, the staleness paths in kernfs_dop_revalidate() never ran, and
> __detach_mounts() was never entered.
> 
> Granting a path to a VE through ve.sysfs_permissions still makes it visible
> and revoking it hides it again, and the host mount now survives the revoke,
> which it did not before.
> 
> ve_perms_test passes 16 of 16 and ve_ns_owner_test 2 of 2, together with the
> filesystems, mount, mount_setattr, move_mount_set_group, nsfs and proc
> selftests. Patch 2 fails on the unpatched kernel with "the VE lookup
> unmounted /sys/power" and passes with patch 1 applied.
> 
> Two of the six ->d_revalidate call sites, __lookup_slow() and lookup_open(),
> were not reached at runtime. This tree carries lookup_fast_for_open(), so even
> an O_CREAT open resolves the last component through lookup_fast(), which
> leaves those two reachable only through a dcache race. Both gate
> d_invalidate() on exactly 0, as do the sites that were exercised,
> ovl_revalidate_real() and ecryptfs_d_revalidate().
> 
> Mirian Shilakadze (2):
>   fs/kernfs, ve: hide entries from a VE without invalidating the dentry
>   selftests/ve: check that hiding an entry does not unmount it
> 
>  fs/kernfs/dir.c                            | 13 +++++-
>  tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++
>  tools/testing/selftests/ve/ve_selftest.h   | 27 ++++++++++-
>  3 files changed, 88 insertions(+), 4 deletions(-)
> 

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.