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

[Pavel Tikhomirov <ptikhomirov@virtuozzo.com>]

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.