Re: [PATCH 07/17] selftests/filesystems: check that OPEN_TREE_NAMESPACE keeps mounts covered

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: open_tree_ns_covered_test fails instead of skipping on unsupported kernels
--

commit a67cc0a2598a800459db0d5c4bf95ad3d7f9463e
Author: Christian Brauner <brauner@kernel.org>

selftests/filesystems: check that OPEN_TREE_NAMESPACE keeps mounts covered

This commit adds a test for open_tree(OPEN_TREE_NAMESPACE) from a user
namespace that does not own the mount namespace it copies from, ensuring
proper behavior of recursive and non-recursive copies.

> diff --git a/tools/testing/selftests/filesystems/open_tree_ns/open_tree_ns_covered_test.c b/tools/testing/selftests/filesystems/open_tree_ns/open_tree_ns_covered_test.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/open_tree_ns/open_tree_ns_covered_test.c
[ ... ]
> +FIXTURE_SETUP(open_tree_ns_covered)
> +{
> +	char p[PATH_LEN];
> +
> +	snprintf(self->dir, sizeof(self->dir), "/tmp/open_tree_ns_covered.XXXXXX");
> +	ASSERT_NE(mkdtemp(self->dir), NULL);
> +	if (enter_userns(getuid(), getgid()) || unshare(CLONE_NEWNS)) {
> +		rmdir(self->dir);
> +		SKIP(return, "test requires user namespaces");
> +	}

[Severity: Medium]
Would it be appropriate to add a capability check here in the
open_tree_ns_covered fixture setup for the open_tree() syscall and the
OPEN_TREE_NAMESPACE flag before continuing?

This would be analogous to the checks performed in open_tree_ns_test.c and
might help avoid false positive failures when automated CI tests run against
older or LTS kernels that lack support.

[ ... ]
> +/* A caller in a new user namespace that doesn't own the mount namespace. */
> +static int foreign_child(const char *dir)
> +{
> +	struct stat st;
> +	int fd;
> +
> +	if (enter_userns(0, 0))
> +		return CHILD_USERNS;
> +
> +	fd = sys_open_tree(AT_FDCWD, dir, OPEN_TREE_NAMESPACE | OPEN_TREE_CLOEXEC);
> +	if (fd >= 0)
> +		return CHILD_NONREC_ALLOWED;
> +	if (errno != EINVAL)
> +		return CHILD_NONREC_ERRNO;
> +
> +	fd = sys_open_tree(AT_FDCWD, dir,
> +			   OPEN_TREE_NAMESPACE | OPEN_TREE_CLOEXEC | AT_RECURSIVE);
> +	if (fd < 0)
> +		return CHILD_REC_REFUSED;

[Severity: Medium]
On older kernels lacking OPEN_TREE_NAMESPACE support (or the open_tree
syscall entirely), will this fail the test instead of gracefully skipping it?

When the flag is unsupported, sys_open_tree() returns -1 with errno set to
EINVAL. The first non-recursive call in this function expects EINVAL, so it
silently passes the check. However, this subsequent recursive call also fails
with EINVAL, which returns CHILD_REC_REFUSED and triggers an assertion failure.

If the open_tree syscall is missing entirely and returns ENOSYS, it appears
it would fail earlier at the non-recursive call, returning CHILD_NONREC_ERRNO.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-work-mount-fixes-3-v1-0-be34c83956ae@kernel.org?part=7