Re: [PATCH 3/9] selftests/exec: test registering an entry disabled

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `removed_by_remove_all` test permanently clears all active `binfmt_misc` entries system-wide, destroying the host system's configuration.
--

--- Patch [3]: [PATCH 3/9] selftests/exec: test registering an entry disabled ---
commit 887542eb0fe41cea7f35d27e339ff39a03f72a8a
Author: Christian Brauner <[email protected]>

selftests/exec: test registering an entry disabled

This commit adds a test for the 'D' flag in binfmt_misc, which allows
registering an entry in a disabled state. It verifies that the entry does
not dispatch until enabled, retains the correct status, and can be toggled
or removed.

> diff --git a/tools/testing/selftests/exec/binfmt_misc_disabled.c b/tools/testing/selftests/exec/binfmt_misc_disabled.c
> new file mode 100644
> index 0000000000000..47c9e8a4ee42a
> --- /dev/null
> +++ b/tools/testing/selftests/exec/binfmt_misc_disabled.c

[ ... ]

> +/* '-1' to the status file sweeps a staged entry with everything else. */
> +TEST_F(disabled, removed_by_remove_all)
> +{
> +	int fd;
> +
> +	ASSERT_EQ(write_reg(RULE("D")), 0);
> +	EXPECT_TRUE(entry_shows(ENTRY, "disabled"));
> +
> +	fd = open(BINFMT_DIR "/status", O_WRONLY | O_CLOEXEC);
> +	ASSERT_GE(fd, 0);
> +	ASSERT_EQ(write(fd, "-1", 2), 2);

[Severity: High]
Could this inadvertently clear the host's global binfmt_misc configuration?

Writing "-1" to the status file triggers a removal of all binfmt_misc
entries system-wide. Since this test requires root privileges to run, but
doesn't appear to call unshare(CLONE_NEWNS) to isolate its mount namespace,
wouldn't this wipe out all active binfmt_misc registrations on the host
system running the selftests?

Other tests in the same directory (like binfmt_misc_selfpin.c) unshare the
namespace before modifying global settings. Does this test need a similar
isolation step to prevent breaking existing host configurations for
utilities like qemu-user-static or WINE?

> +	close(fd);
> +
> +	EXPECT_NE(access(BINFMT_DIR "/" ENTRY, F_OK), 0);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3