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

[Devel] [PATCH vz10 v2 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 reports the name as missing from that check, except on a kernfs
instance the VE created, where the dentry is dropped as before. Everywhere
else, the host's sysfs above all, 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
either way: the errno for a hidden entry is ENOENT, 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".

The one exception is a create attempt on a hidden name, and only on an
instance the VE did not mount, where it now fails with ENOENT rather than
the EACCES it fails with today. On the VE's own instance nothing changes, a
create on a hidden name still fails with EACCES. Both fail either way.

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.

The condition added in v2 was checked on both sides. During a container
start it never fires: of the 13 lookups that answered 0, every one returned
before reaching the visibility check, from the negative dentry branch or
from !kernfs_active(), which are the device mapper and uevent nodes churning
as the disk is set up. The lookups that do reach the check answer ENOENT, 8
of them, and __detach_mounts() is not called at all. It fires where it is
meant to: a container looking up a hidden entry in its own sysfs instance
gets the dentry dropped and the mount on it detached, while the same lookup
against the host's sysfs answers ENOENT and leaves the mount alone.

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

v2:
 - patch 1: keep the old invalidate on a kernfs instance the VE created,
   and only report the name as missing on any other instance (Pavel)
 - patch 2: detect the mount with openat2(RESOLVE_NO_XDEV) rather than
   reading /proc/self/mountinfo (Pavel)
 - dropped Pavel's Reviewed-by from v1, both patches changed

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                            | 17 ++++++-
 tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++
 tools/testing/selftests/ve/ve_selftest.h   | 40 ++++++++++++++++-
 3 files changed, 105 insertions(+), 4 deletions(-)

-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH vz10 v2 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.

Report the name as missing instead, except on a kernfs instance that
this VE created, where the dentry is dropped as before.  Everywhere else,
the host's sysfs above all, 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 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index be680eb98ed4..4a5ee299a94e 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1199,8 +1199,21 @@ 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))) {
+		/*
+		 * On an instance this VE created, drop the dentry as before.
+		 * Anywhere else the node is fine and 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.
+		 */
+		if (kernfs_info(dentry->d_sb)->ve == get_exec_env())
+			goto out_bad;
+		up_read(&root->kernfs_rwsem);
+		return -ENOENT;
+	}
 
 	up_read(&root->kernfs_rwsem);
 	return 1;
-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

[Devel] [PATCH vz10 v2 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.

The mount check uses openat2() with RESOLVE_NO_XDEV, which fails with
EXDEV when the final component is a mount point, rather than reading
/proc/self/mountinfo.

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   | 40 ++++++++++++++++-
 2 files changed, 90 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..c53bf7900d20 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
@@ -16,6 +16,8 @@
 #include <limits.h>
 #include <sys/stat.h>
 #include <sys/mount.h>
+#include <sys/syscall.h>
+#include <linux/openat2.h>
 
 #ifndef CLONE_NEWVE
 #define CLONE_NEWVE		0x00000040
@@ -180,4 +182,38 @@ static inline void destroy_ve(int cgv2_fd, int id)
 		__func__, id, strerror(errno));
 }
 
+/*
+ * Is @path a mount point?  RESOLVE_NO_XDEV makes openat2() fail with EXDEV
+ * when the final component is a mount point, which answers the question
+ * without reading the mount table.
+ */
+static inline int is_mounted(const char *path)
+{
+	struct open_how how = {
+		.flags = O_PATH | O_CLOEXEC,
+		.resolve = RESOLVE_NO_XDEV,
+	};
+	char buf[PATH_MAX], *dir, *base;
+	int dfd, fd;
+
+	if (snprintf(buf, sizeof(buf), "%s", path) >= (int)sizeof(buf))
+		return -1;
+	base = strrchr(buf, '/');
+	if (!base)
+		return -1;
+	*base++ = '\0';
+	dir = buf[0] ? buf : "/";
+
+	dfd = open(dir, O_PATH | O_DIRECTORY | O_CLOEXEC);
+	if (dfd < 0)
+		return -1;
+	fd = syscall(__NR_openat2, dfd, base, &how, sizeof(how));
+	close(dfd);
+	if (fd >= 0) {
+		close(fd);
+		return 0;
+	}
+	return errno == EXDEV ? 1 : -1;
+}
+
 #endif /* __SELFTESTS_VE_VE_SELFTEST_H */
-- 
2.43.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Pavel Tikhomirov]

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

On 8/26/26 13:04, 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 reports the name as missing from that check, except on a kernfs
> instance the VE created, where the dentry is dropped as before. Everywhere
> else, the host's sysfs above all, 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
> either way: the errno for a hidden entry is ENOENT, 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".
> 
> The one exception is a create attempt on a hidden name, and only on an
> instance the VE did not mount, where it now fails with ENOENT rather than
> the EACCES it fails with today. On the VE's own instance nothing changes, a
> create on a hidden name still fails with EACCES. Both fail either way.
> 
> 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.
> 
> The condition added in v2 was checked on both sides. During a container
> start it never fires: of the 13 lookups that answered 0, every one returned
> before reaching the visibility check, from the negative dentry branch or
> from !kernfs_active(), which are the device mapper and uevent nodes churning
> as the disk is set up. The lookups that do reach the check answer ENOENT, 8
> of them, and __detach_mounts() is not called at all. It fires where it is
> meant to: a container looking up a hidden entry in its own sysfs instance
> gets the dentry dropped and the mount on it detached, while the same lookup
> against the host's sysfs answers ENOENT and leaves the mount alone.
> 
> 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().
> 
> v2:
>  - patch 1: keep the old invalidate on a kernfs instance the VE created,
>    and only report the name as missing on any other instance (Pavel)
>  - patch 2: detect the mount with openat2(RESOLVE_NO_XDEV) rather than
>    reading /proc/self/mountinfo (Pavel)
>  - dropped Pavel's Reviewed-by from v1, both patches changed
> 
> 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                            | 17 ++++++-
>  tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++
>  tools/testing/selftests/ve/ve_selftest.h   | 40 ++++++++++++++++-
>  3 files changed, 105 insertions(+), 4 deletions(-)
> 

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

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Konstantin Khorenko]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.10.vz10
------>
commit e6d9a8ea97c1933241869f60ac5677865d848dab
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Wed Aug 26 15:04:11 2026 +0400

    selftests/ve: check that hiding an entry does not unmount it
    
    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.
    
    The mount check uses openat2() with RESOLVE_NO_XDEV, which fails with
    EXDEV when the final component is a mount point, rather than reading
    /proc/self/mountinfo.
    
    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>
    Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 tools/testing/selftests/ve/ve_perms_test.c | 52 ++++++++++++++++++++++++++++++
 tools/testing/selftests/ve/ve_selftest.h   | 40 +++++++++++++++++++++--
 2 files changed, 90 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/ve/ve_perms_test.c b/tools/testing/selftests/ve/ve_perms_test.c
index 4522950c17f2f..25ffbd42c3802 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 69c0a52dd7ef0..c53bf7900d208 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
@@ -16,6 +16,8 @@
 #include <limits.h>
 #include <sys/stat.h>
 #include <sys/mount.h>
+#include <sys/syscall.h>
+#include <linux/openat2.h>
 
 #ifndef CLONE_NEWVE
 #define CLONE_NEWVE		0x00000040
@@ -180,4 +182,38 @@ static inline void destroy_ve(int cgv2_fd, int id)
 		__func__, id, strerror(errno));
 }
 
+/*
+ * Is @path a mount point?  RESOLVE_NO_XDEV makes openat2() fail with EXDEV
+ * when the final component is a mount point, which answers the question
+ * without reading the mount table.
+ */
+static inline int is_mounted(const char *path)
+{
+	struct open_how how = {
+		.flags = O_PATH | O_CLOEXEC,
+		.resolve = RESOLVE_NO_XDEV,
+	};
+	char buf[PATH_MAX], *dir, *base;
+	int dfd, fd;
+
+	if (snprintf(buf, sizeof(buf), "%s", path) >= (int)sizeof(buf))
+		return -1;
+	base = strrchr(buf, '/');
+	if (!base)
+		return -1;
+	*base++ = '\0';
+	dir = buf[0] ? buf : "/";
+
+	dfd = open(dir, O_PATH | O_DIRECTORY | O_CLOEXEC);
+	if (dfd < 0)
+		return -1;
+	fd = syscall(__NR_openat2, dfd, base, &how, sizeof(how));
+	close(dfd);
+	if (fd >= 0) {
+		close(fd);
+		return 0;
+	}
+	return errno == EXDEV ? 1 : -1;
+}
+
 #endif /* __SELFTESTS_VE_VE_SELFTEST_H */
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Re: [Devel] [PATCH RHEL10 COMMIT] fs/kernfs, ve: hide entries from a VE without invalidating the dentry

[Konstantin Khorenko]

The commit is pushed to "branch-rh10-6.12.0-211.39.1.16.x.vz10-ovz" and will appear at git@bitbucket.org:openvz/vzkernel.git
after rh10-6.12.0-211.39.1.16.10.vz10
------>
commit d57b4b66477bc8f4afed0a532fc40ee19f31e286
Author: Mirian Shilakadze <mirian.shilakadze@virtuozzo.com>
Date:   Wed Aug 26 15:04:10 2026 +0400

    fs/kernfs, ve: hide entries from a VE without invalidating the dentry
    
    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.
    
    Report the name as missing instead, except on a kernfs instance that
    this VE created, where the dentry is dropped as before.  Everywhere else,
    the host's sysfs above all, 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>
    Reviewed-by: Pavel Tikhomirov <ptikhomirov@virtuozzo.com>
    Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
---
 fs/kernfs/dir.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index be680eb98ed4f..4a5ee299a94eb 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1199,8 +1199,21 @@ 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))) {
+		/*
+		 * On an instance this VE created, drop the dentry as before.
+		 * Anywhere else the node is fine and 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.
+		 */
+		if (kernfs_info(dentry->d_sb)->ve == get_exec_env())
+			goto out_bad;
+		up_read(&root->kernfs_rwsem);
+		return -ENOENT;
+	}
 
 	up_read(&root->kernfs_rwsem);
 	return 1;
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

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

[Konstantin Khorenko]

On 8/26/26 13:04, Mirian Shilakadze wrote:
...
> diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> index be680eb98ed4..4a5ee299a94e 100644
> --- a/fs/kernfs/dir.c
> +++ b/fs/kernfs/dir.c
> @@ -1199,8 +1199,21 @@ 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))) {
> +		/*
> +		 * On an instance this VE created, drop the dentry as before.
> +		 * Anywhere else the node is fine and 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.
> +		 */
> +		if (kernfs_info(dentry->d_sb)->ve == get_exec_env())

kernfs of a VE may outlive the VE (and ve_struct) - if kernefs of a CT is pinned somehow while CT is stopped,
there is no get_ve() or any other blocker for a Container destruction before VE kernfs is unmounted.

And in case the VE1 died and VE2 is started and occasionally ve_struct got VE2 gets the same pointer,
then the comparison will tell us not truth.

=======
  What is stored in info->ve

  When some VE mounts its own kernfs instance (for example, a container mounts its own sysfs), kernfs_get_tree() creates a
  new superblock and a pointer to the mounting task's VE is written into its kernfs_super_info (fs/kernfs/mount.c:365):

  info->ve = get_exec_env();

  The key point: this is a raw pointer. There is no get_ve(), no taking of any other reference there. That is, the
  superblock does not in any way extend the lifetime of the ve_struct it points to. This was the case before the patch too -
  patch 1 merely added a second place where this pointer is used (the comparison in kernfs_dop_revalidate()).

  How a superblock can outlive its VE

  Normally a container's sysfs is unmounted when the container stops, and the sb dies together with it. But the mount can be
  "pinned" from outside: for example, the host bind-mounted the container's sysfs somewhere into its own mount namespace,
  or some process holds an open fd inside that mount. Then, after the container is stopped and destroyed, its ve_struct is
  freed (kfree), while the superblock, with info->ve pointing at already freed memory, lives on.

  By itself this is not a use-after-free: the pointer is never dereferenced anywhere, it is only compared with
  get_exec_env(). Comparing addresses with a freed object is legal - nobody tries to read through the address.

  Where the trap appears: address reuse

  kfree() returns the memory to the allocator, and the next kmalloc() of the same size may well hand out the very same
  address. A new container starts - its ve_struct may end up at the address where the dead container's ve_struct used to
  live.

  Now the scenario in full:

  1. Container A mounts its own sysfs -> an sb with info->ve = <address X>.
  2. The host pinned that mount, container A is destroyed, the ve_struct at address X is freed. The sb is alive, info->ve =
     X (dangling).
  3. Container B starts, its ve_struct is allocated at the same address X.
  4. A task of container B, by some path (through that host-pinned mount), does a lookup of an entry hidden from it on this
     old sb.
  5. The check kernfs_info(dentry->d_sb)->ve == get_exec_env() gives X == X -> true, even though container B did not create
     this instance. The code wrongly decides "this is this VE's own instance" and takes the old behavior: goto out_bad ->
     return 0 -> d_invalidate() instead of the new -ENOENT.

  Why I don't consider this a blocker

  The consequence of the false match is merely a fallback to today's (pre-patch) behavior: the dentry is invalidated and the
  mounts on that dentry are detached. But the dentry belongs to the sysfs instance of dead container A, that is, the only
  things that can suffer are mounts placed on top of entries of that dead instance - not the host's /sys and not container
  B's sysfs. Plus the scenario itself requires exotics: the host is for some reason holding a dead container's sysfs, the
  new VE landed at exactly the same address, and its task is walking that foreign mount.

  The reverse direction is safe automatically: "didn't match although it should have" is impossible, because the VE that
  actually created the instance is dead - it has no tasks left, there is nothing to compare against. The only possible
  failure is a false match, and, as shown above, it degrades into the old behavior, not into a new hole.

  That is why the wording was "worth being aware": the patch makes a long-existing weak spot (a pointer not backed by a
  reference) matter for the logic (load-bearing) for the first time, but no real harm can be extracted from it. If one
  wanted to close the question nicely, one could store not the pointer but, say, ve->veid, or take a get_ve() reference when
  setting info->ve and drop it in kernfs_free_fs_context/on sb destruction. But I would not demand that from this series.


> +			goto out_bad;
> +		up_read(&root->kernfs_rwsem);
> +		return -ENOENT;
> +	}
>  
>  	up_read(&root->kernfs_rwsem);
>  	return 1;

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel