Re: [PATCH 09/17] selftests/filesystems: check that a mount namespace file on top doesn't bury a mount

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests: test hangs if child unshare() fails due to unclosed pipe
--

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

selftests/filesystems: check that a mount namespace file on top doesn't bury a mount

This commit adds a test for moving a mount with a mount namespace file on
top of it. It ensures that when a mount is moved onto a shared mount, its
copy propagates correctly to slaves without being buried.

> diff --git a/tools/testing/selftests/filesystems/mount_cycle/overmount_ns_file_test.c b/tools/testing/selftests/filesystems/mount_cycle/overmount_ns_file_test.c
> new file mode 100644
> index 0000000000000..c24436a17c0fd
> --- /dev/null
> +++ b/tools/testing/selftests/filesystems/mount_cycle/overmount_ns_file_test.c
> @@ -0,0 +1,187 @@

[ ... ]

> +TEST_F(overmount_ns_file, existing_mount_stays_on_top)
> +{

[ ... ]

> +	/* a newer mount namespace whose file can be bound */
> +	ASSERT_EQ(pipe(pfd), 0);
> +	self->child = fork();
> +	ASSERT_GE(self->child, 0);
> +	if (self->child == 0) {
> +		if (unshare(CLONE_NEWNS) || write(pfd[1], "r", 1) != 1)
> +			_exit(1);
> +		pause();
> +		_exit(0);
> +	}
> +	ASSERT_EQ(read(pfd[0], &c, 1), 1);

[Severity: Medium]
Will this test hang if unshare(CLONE_NEWNS) fails in the child process?

If the child process exits via _exit(1) due to unshare() failing, it closes
its file descriptors. But since the parent process never closed its own copy
of the pipe's write end (pfd[1]), the pipe is not considered closed.

The read() call here will block forever waiting for data instead of returning
an EOF, causing the test program to hang indefinitely.

Should the parent close(pfd[1]) before attempting to read from pfd[0]?

> +	snprintf(s, sizeof(s), "/proc/%d/ns/mnt", self->child);
> +	ASSERT_EQ(mount(s, p, NULL, MS_BIND, NULL), 0);
> +
> +	ASSERT_STREQ(read_file(b, buf, sizeof(buf)), "Q");

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