All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Christian Brauner" <brauner@kernel.org>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH 3/9] selftests/exec: test registering an entry disabled
Date: Thu, 30 Jul 2026 13:45:44 +0000	[thread overview]
Message-ID: <20260730134545.451731F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260730-work-binfmt_misc-preopen-v1-3-4a0b0da71f16@kernel.org>

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 <brauner@kernel.org>

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/20260730-work-binfmt_misc-preopen-v1-0-4a0b0da71f16@kernel.org?part=3

  reply	other threads:[~2026-07-30 13:45 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-30 13:34 [PATCH 0/9] binfmt_misc: bind interpreters to a bpf-backed entry Christian Brauner
2026-07-30 13:34 ` [PATCH 1/9] binfmt_misc: let a register string create an entry disabled Christian Brauner
2026-07-30 13:47   ` sashiko-bot
2026-07-30 13:34 ` [PATCH 2/9] selftests/exec: let binfmt_flag_supported() return a bool Christian Brauner
2026-07-30 13:34 ` [PATCH 3/9] selftests/exec: test registering an entry disabled Christian Brauner
2026-07-30 13:45   ` sashiko-bot [this message]
2026-07-30 13:34 ` [PATCH 4/9] binfmt_misc: document " Christian Brauner
2026-07-30 13:34 ` [PATCH 5/9] selftests/exec: share the bpf handler preconditions Christian Brauner
2026-07-30 13:34 ` [PATCH 6/9] binfmt_misc: carry pre-opened interpreters in struct binfmt_misc_interp Christian Brauner
2026-07-30 13:34 ` [PATCH 7/9] binfmt_misc: let a 'B' entry bind its interpreters Christian Brauner
2026-07-30 13:34 ` [PATCH 8/9] selftests/exec: test interpreters bound to a 'B' entry Christian Brauner
2026-07-30 13:34 ` [PATCH 9/9] binfmt_misc: document interpreters bound by " Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260730134545.451731F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.