From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pavel Tikhomirov Date: Tue, 25 Aug 2026 17:40:27 +0200 Subject: Re: [Devel] [PATCH vz10 2/2] selftests/ve: check that hiding an entry does not unmount it In-Reply-To: <20260825133013.704924-3-mirian.shilakadze@virtuozzo.com> References: <20260825133013.704924-1-mirian.shilakadze@virtuozzo.com> <20260825133013.704924-3-mirian.shilakadze@virtuozzo.com> Message-ID: <1e09adbd-f820-482b-9c48-814e4d62f5be@virtuozzo.com> List-Id: 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 > --- > 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 > #include > #include > +#include > #include > #include > > @@ -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.