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

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

[Mirian Shilakadze]

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(-)

-- 
2.43.0

[Devel] [PATCH vz10 1/2] fs/kernfs, ve: hide entries from a VE without invalidating the dentry

[Mirian Shilakadze]

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 walks
the subtree and hands every mountpoint it finds to __detach_mounts().
The mountpoint hash is not scoped to a mount namespace, and m_list holds
every mount attached at that dentry in any of them, so a Container's
lookup unmounts the host's mounts.

One lookup of /sys/fs/bpf from a task that only did setns() into a
Container's ve namespace, staying in the host mount namespace, both
hides the entry from the caller and destroys the host's bpffs.  A
Container start reaches the same path on its own: libvzctl stats every
mount point in the namespace to collect the mount flags of a bindmount
source, and does it after CLONE_NEWVE and before pivot_root, so the host
loses bpffs and tracefs on the way.  libvzctl needs bpffs for the cgroup
v2 device controller, so no Container on the node can be managed
afterwards, and the damage outlives the failed start.

Return -ENOENT instead.  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.  No caller of ->d_revalidate() reaches
d_invalidate() with a negative return: lookup_dcache(), lookup_fast(),
__lookup_slow() and lookup_open() in fs/namei.c all gate it on exactly
0, ovl_revalidate_real() gates it the same way, and
ecryptfs_d_revalidate() hands the value back without invalidating
anything itself.  kernfs_iop_lookup() already answers this same
condition with a plain "not found".

Feature: kernfs: per-CT entries visibility and permissions configuration
https://virtuozzo.atlassian.net/browse/VSTOR-142552
Fixes: 3dd8c2499df6 ("ve/kernfs: hide forbidden entries in container")
Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
 fs/kernfs/dir.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index be680eb98ed4..7947c49ed1a6 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1199,8 +1199,17 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
 	    kernfs_info(dentry->d_sb)->ns != kn->ns)
 		goto out_bad;
 
-	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb)))
-		goto out_bad;
+	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb))) {
+		/*
+		 * The node is fine, it is only outside this VE's view.
+		 * Returning 0 would tell the VFS that the dentry is stale, and
+		 * it answers that with d_invalidate(), which detaches every
+		 * mount on that dentry in every mount namespace.  Report the
+		 * name as missing to this caller instead.
+		 */
+		up_read(&root->kernfs_rwsem);
+		return -ENOENT;
+	}
 
 	up_read(&root->kernfs_rwsem);
 	return 1;
-- 
2.43.0

[Devel] [PATCH vz10 2/2] selftests/ve: check that hiding an entry does not unmount it

[Mirian Shilakadze]

kernfs_dop_revalidate() answered the per VE visibility check with the same
"return 0" the staleness checks use, and the VFS reads 0 as a global fact:
d_invalidate() hands every mountpoint under that dentry to
__detach_mounts(), whose mountpoint hash is not scoped to a mount
namespace.  A single lookup from inside a Container unmounted the host's
bpffs, and libvzctl needs bpffs for the cgroup v2 device controller, so
the whole node stopped being manageable.

Mount a tmpfs on the entry the variant already keeps host only, look it up
from inside a VE, and require both that the VE is told ENOENT and that the
mount is still there afterwards.  The mount is made in the test's own
mount namespace so the machine running the test cannot lose a mount it
needs, while the dentry the mount hangs on is still the shared one the bug
worked through.

Fails without the preceding fix, on both the sysfs and the proc variant.

Feature: kernfs: per-CT entries visibility and permissions configuration
https://virtuozzo.atlassian.net/browse/VSTOR-142552
Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++
 tools/testing/selftests/ve/ve_selftest.h   | 27 ++++++++++-
 2 files changed, 77 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_perms_test.c b/tools/testing/selftests/ve/ve_perms_test.c
index 4522950c17f2..25ffbd42c380 100644
--- a/tools/testing/selftests/ve/ve_perms_test.c
+++ b/tools/testing/selftests/ve/ve_perms_test.c
@@ -24,6 +24,7 @@
 #include <unistd.h>
 #include <fcntl.h>
 #include <limits.h>
+#include <sys/mount.h>
 #include <sys/wait.h>
 #include <errno.h>
 
@@ -412,4 +413,55 @@ TEST_F(ve_perms, enforce_denies)
 			      absent, O_RDONLY), EACCES);
 }
 
+/*
+ * Looking up an entry that a VE cannot see must not disturb a mount that
+ * sits on it.
+ *
+ * The lookup used to answer "this dentry is stale" where it meant "this name
+ * is not here for you", and the VFS acts on stale globally: d_invalidate()
+ * detaches every mount on that dentry in every mount namespace.  One lookup
+ * from inside a Container took the host's bpffs and tracefs with it.
+ *
+ * The tmpfs is mounted in the test's own mount namespace, so the machine
+ * running this cannot lose a mount it needs, while the dentry the mount hangs
+ * on is still the shared one the bug worked through.
+ */
+TEST_F(ve_perms, hidden_entry_keeps_its_mount)
+{
+	char path[PATH_MAX];
+	int status;
+	pid_t pid;
+
+	if (!entry_present(variant->dir_prefix, variant->dir))
+		SKIP(return, "%s/%s absent", variant->dir_prefix, variant->dir);
+	snprintf(path, sizeof(path), "%s/%s", variant->dir_prefix, variant->dir);
+
+	pid = fork();
+	ASSERT_GE(pid, 0);
+	if (pid == 0) {
+		if (unshare(CLONE_NEWNS) != 0 ||
+		    mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0 ||
+		    mount("ve_selftest", path, "tmpfs", 0, NULL) != 0)
+			_exit(255);
+		if (is_mounted(path) != 1)
+			_exit(254);
+
+		/*
+		 * The lookup that used to unmount it.  What the VE is told
+		 * depends on the filesystem and on the mount now covering the
+		 * entry, and enforce_denies() already covers that.  Here only
+		 * the mount surviving the lookup is the point.
+		 */
+		ve_open_rel(self->cgv2_fd, self->ctid_a, variant->dir_prefix,
+			    variant->dir, O_RDONLY | O_DIRECTORY);
+
+		_exit(is_mounted(path) == 1 ? 0 : 2);
+	}
+	ASSERT_EQ(waitpid(pid, &status, 0), pid);
+	ASSERT_TRUE(WIFEXITED(status));
+	if (WEXITSTATUS(status) == 2)
+		TH_LOG("the VE lookup unmounted %s", path);
+	EXPECT_EQ(WEXITSTATUS(status), 0);
+}
+
 TEST_HARNESS_MAIN
diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
index 69c0a52dd7ef..83ace9e6db42 100644
--- a/tools/testing/selftests/ve/ve_selftest.h
+++ b/tools/testing/selftests/ve/ve_selftest.h
@@ -1,8 +1,8 @@
 /* SPDX-License-Identifier: GPL-2.0 */
 /*
  * Shared helpers for the ve selftests: a private cgroup2 mount, small file and
- * cgroup helpers, and VE cgroup create and destroy, used across the tests in
- * this directory.
+ * cgroup helpers, VE cgroup create and destroy, and a mount point query, used
+ * across the tests in this directory.
  */
 #ifndef __SELFTESTS_VE_VE_SELFTEST_H
 #define __SELFTESTS_VE_VE_SELFTEST_H
@@ -180,4 +180,27 @@ static inline void destroy_ve(int cgv2_fd, int id)
 		__func__, id, strerror(errno));
 }
 
+/* Is @path a mount point in this task's mount namespace? */
+static inline int is_mounted(const char *path)
+{
+	char line[PATH_MAX + 128], target[PATH_MAX];
+	int found = 0;
+	unsigned int u;
+	FILE *f;
+
+	f = fopen("/proc/self/mountinfo", "r");
+	if (!f)
+		return -1;
+	while (fgets(line, sizeof(line), f)) {
+		if (sscanf(line, "%u %u %u:%u %*s %s", &u, &u, &u, &u, target) != 5)
+			continue;
+		if (strcmp(target, path) == 0) {
+			found = 1;
+			break;
+		}
+	}
+	fclose(f);
+	return found;
+}
+
 #endif /* __SELFTESTS_VE_VE_SELFTEST_H */
-- 
2.43.0

[Devel] [PATCH vz10 1/2] fs/kernfs, ve: hide entries from a VE without invalidating the dentry

[Pavel Tikhomirov]

On 8/25/26 15:29, Mirian Shilakadze wrote:
> 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 walks
> the subtree and hands every mountpoint it finds to __detach_mounts().
> The mountpoint hash is not scoped to a mount namespace, and m_list holds
> every mount attached at that dentry in any of them, so a Container's
> lookup unmounts the host's mounts.
> 
> One lookup of /sys/fs/bpf from a task that only did setns() into a
> Container's ve namespace, staying in the host mount namespace, both
> hides the entry from the caller and destroys the host's bpffs.  A
> Container start reaches the same path on its own: libvzctl stats every
> mount point in the namespace to collect the mount flags of a bindmount
> source, and does it after CLONE_NEWVE and before pivot_root, so the host
> loses bpffs and tracefs on the way.  libvzctl needs bpffs for the cgroup
> v2 device controller, so no Container on the node can be managed
> afterwards, and the damage outlives the failed start.
> 
> Return -ENOENT instead.  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.  No caller of ->d_revalidate() reaches
> d_invalidate() with a negative return: lookup_dcache(), lookup_fast(),
> __lookup_slow() and lookup_open() in fs/namei.c all gate it on exactly
> 0, ovl_revalidate_real() gates it the same way, and
> ecryptfs_d_revalidate() hands the value back without invalidating
> anything itself.  kernfs_iop_lookup() already answers this same
> condition with a plain "not found".
> 
> Feature: kernfs: per-CT entries visibility and permissions configuration
> https://virtuozzo.atlassian.net/browse/VSTOR-142552
> Fixes: 3dd8c2499df6 ("ve/kernfs: hide forbidden entries in container")
> Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
> ---
>  fs/kernfs/dir.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index be680eb98ed4..7947c49ed1a6 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1199,8 +1199,17 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)
>  	    kernfs_info(dentry->d_sb)->ns != kn->ns)
>  		goto out_bad;
>  
> -	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb)))
> -		goto out_bad;
> +	if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb))) {
> +		/*
> +		 * The node is fine, it is only outside this VE's view.
> +		 * Returning 0 would tell the VFS that the dentry is stale, and
> +		 * it answers that with d_invalidate(), which detaches every
> +		 * mount on that dentry in every mount namespace.  Report the
> +		 * name as missing to this caller instead.
> +		 */
> +		up_read(&root->kernfs_rwsem);
> +		return -ENOENT;
> +	}

Not fully sure, but probably worth preserving behavior in CT as much as possible:

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 7947c49ed1a6f..daa4bc264e6bb 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1201,12 +1201,11 @@ static int kernfs_dop_revalidate(struct dentry *dentry, unsigned int flags)

        if (!kernfs_d_visible(kn, kernfs_info(dentry->d_sb))) {
                /*
-                * The node is fine, it is only outside this VE's view.
-                * Returning 0 would tell the VFS that the dentry is stale, and
-                * it answers that with d_invalidate(), which detaches every
-                * mount on that dentry in every mount namespace.  Report the
-                * name as missing to this caller instead.
+                * Invalidate the node in ve owned mount namespace,
+                * or report no entry in other mount namespace.
                 */
+               if (current->nsproxy->mnt_ns->ve_owner == get_exec_env())
+                       return 0;
                up_read(&root->kernfs_rwsem);
                return -ENOENT;
        }

This way we still umount and invalidate dentry in CT filesystem, making it look a bit more clean.

>  
>  	up_read(&root->kernfs_rwsem);
>  	return 1;

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

Re: [Devel] [PATCH vz10 2/2] selftests/ve: check that hiding an entry does not unmount it

[Pavel Tikhomirov]

On 8/25/26 15:29, Mirian Shilakadze wrote:
> kernfs_dop_revalidate() answered the per VE visibility check with the same
> "return 0" the staleness checks use, and the VFS reads 0 as a global fact:
> d_invalidate() hands every mountpoint under that dentry to
> __detach_mounts(), whose mountpoint hash is not scoped to a mount
> namespace.  A single lookup from inside a Container unmounted the host's
> bpffs, and libvzctl needs bpffs for the cgroup v2 device controller, so
> the whole node stopped being manageable.
> 
> Mount a tmpfs on the entry the variant already keeps host only, look it up
> from inside a VE, and require both that the VE is told ENOENT and that the
> mount is still there afterwards.  The mount is made in the test's own
> mount namespace so the machine running the test cannot lose a mount it
> needs, while the dentry the mount hangs on is still the shared one the bug
> worked through.
> 
> Fails without the preceding fix, on both the sysfs and the proc variant.
> 
> Feature: kernfs: per-CT entries visibility and permissions configuration
> https://virtuozzo.atlassian.net/browse/VSTOR-142552
> Signed-off-by: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
> ---
>  tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++
>  tools/testing/selftests/ve/ve_selftest.h   | 27 ++++++++++-
>  2 files changed, 77 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/ve/ve_perms_test.c b/tools/testing/selftests/ve/ve_perms_test.c
> index 4522950c17f2..25ffbd42c380 100644
> --- a/tools/testing/selftests/ve/ve_perms_test.c
> +++ b/tools/testing/selftests/ve/ve_perms_test.c
> @@ -24,6 +24,7 @@
>  #include <unistd.h>
>  #include <fcntl.h>
>  #include <limits.h>
> +#include <sys/mount.h>
>  #include <sys/wait.h>
>  #include <errno.h>
>  
> @@ -412,4 +413,55 @@ TEST_F(ve_perms, enforce_denies)
>  			      absent, O_RDONLY), EACCES);
>  }
>  
> +/*
> + * Looking up an entry that a VE cannot see must not disturb a mount that
> + * sits on it.
> + *
> + * The lookup used to answer "this dentry is stale" where it meant "this name
> + * is not here for you", and the VFS acts on stale globally: d_invalidate()
> + * detaches every mount on that dentry in every mount namespace.  One lookup
> + * from inside a Container took the host's bpffs and tracefs with it.
> + *
> + * The tmpfs is mounted in the test's own mount namespace, so the machine
> + * running this cannot lose a mount it needs, while the dentry the mount hangs
> + * on is still the shared one the bug worked through.
> + */
> +TEST_F(ve_perms, hidden_entry_keeps_its_mount)
> +{
> +	char path[PATH_MAX];
> +	int status;
> +	pid_t pid;
> +
> +	if (!entry_present(variant->dir_prefix, variant->dir))
> +		SKIP(return, "%s/%s absent", variant->dir_prefix, variant->dir);
> +	snprintf(path, sizeof(path), "%s/%s", variant->dir_prefix, variant->dir);
> +
> +	pid = fork();
> +	ASSERT_GE(pid, 0);
> +	if (pid == 0) {
> +		if (unshare(CLONE_NEWNS) != 0 ||
> +		    mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL) != 0 ||
> +		    mount("ve_selftest", path, "tmpfs", 0, NULL) != 0)
> +			_exit(255);
> +		if (is_mounted(path) != 1)
> +			_exit(254);
> +
> +		/*
> +		 * The lookup that used to unmount it.  What the VE is told
> +		 * depends on the filesystem and on the mount now covering the
> +		 * entry, and enforce_denies() already covers that.  Here only
> +		 * the mount surviving the lookup is the point.
> +		 */
> +		ve_open_rel(self->cgv2_fd, self->ctid_a, variant->dir_prefix,
> +			    variant->dir, O_RDONLY | O_DIRECTORY);
> +
> +		_exit(is_mounted(path) == 1 ? 0 : 2);
> +	}
> +	ASSERT_EQ(waitpid(pid, &status, 0), pid);
> +	ASSERT_TRUE(WIFEXITED(status));
> +	if (WEXITSTATUS(status) == 2)
> +		TH_LOG("the VE lookup unmounted %s", path);
> +	EXPECT_EQ(WEXITSTATUS(status), 0);
> +}
> +
>  TEST_HARNESS_MAIN
> diff --git a/tools/testing/selftests/ve/ve_selftest.h b/tools/testing/selftests/ve/ve_selftest.h
> index 69c0a52dd7ef..83ace9e6db42 100644
> --- a/tools/testing/selftests/ve/ve_selftest.h
> +++ b/tools/testing/selftests/ve/ve_selftest.h
> @@ -1,8 +1,8 @@
>  /* SPDX-License-Identifier: GPL-2.0 */
>  /*
>   * Shared helpers for the ve selftests: a private cgroup2 mount, small file and
> - * cgroup helpers, and VE cgroup create and destroy, used across the tests in
> - * this directory.
> + * cgroup helpers, VE cgroup create and destroy, and a mount point query, used
> + * across the tests in this directory.
>   */
>  #ifndef __SELFTESTS_VE_VE_SELFTEST_H
>  #define __SELFTESTS_VE_VE_SELFTEST_H
> @@ -180,4 +180,27 @@ static inline void destroy_ve(int cgv2_fd, int id)
>  		__func__, id, strerror(errno));
>  }
>  
> +/* Is @path a mount point in this task's mount namespace? */
> +static inline int is_mounted(const char *path)
> +{
> +	char line[PATH_MAX + 128], target[PATH_MAX];
> +	int found = 0;
> +	unsigned int u;
> +	FILE *f;
> +
> +	f = fopen("/proc/self/mountinfo", "r");
> +	if (!f)
> +		return -1;
> +	while (fgets(line, sizeof(line), f)) {
> +		if (sscanf(line, "%u %u %u:%u %*s %s", &u, &u, &u, &u, target) != 5)
> +			continue;
> +		if (strcmp(target, path) == 0) {
> +			found = 1;
> +			break;
> +		}
> +	}

Take a look at openat2 RESOLVE_NO_XDEV, it detects the crossing of mount boundary
(open parent, open child from parent), and avoids heavy mountinfo reading.

> +	fclose(f);
> +	return found;
> +}
> +
>  #endif /* __SELFTESTS_VE_VE_SELFTEST_H */

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

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

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov@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.