From: sashiko-bot@kernel.org
To: "Christian Brauner" <brauner@kernel.org>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH 01/21] exec: do not act on a stale execfd request without an executable
Date: Mon, 20 Jul 2026 10:00:22 +0000 [thread overview]
Message-ID: <20260720100022.9212B1F00A3A@smtp.kernel.org> (raw)
In-Reply-To: <20260720-work-bpf-binfmt_misc-ptinterp-v1-1-ddb76c9a508e@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch fails to fix it completely. It fails to clear leaked `have_execfd` and `execfd_creds` flags if the fallback format uses an interpreter (e.g., `binfmt_script`), potentially allowing setuid script execution.
--
--- Patch [1]: [PATCH 01/21] exec: do not act on a stale execfd request without an executable ---
commit 9dcfa7daaba6407f79aa884e4fa9f0bc5d755870
Author: Christian Brauner <brauner@kernel.org>
exec: do not act on a stale execfd request without an executable
This commit addresses an issue where binfmt_misc leaves bprm->have_execfd
and bprm->execfd_creds set when it fails to open an interpreter. It clears
these flags in begin_new_exec() if bprm->executable is NULL to prevent a
subsequent NULL pointer dereference.
> diff --git a/fs/exec.c b/fs/exec.c
> index c698aabe9abd0..2f50aa0398278 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -1112,6 +1112,12 @@ int begin_new_exec(struct linux_binprm * bprm)
> struct task_struct *me = current;
> int retval;
>
> + /* A declined execfd request has no executable for a later format. */
> + if (!bprm->executable) {
> + bprm->have_execfd = 0;
> + bprm->execfd_creds = 0;
> + }
[Severity: High]
This isn't a bug introduced by this patch, but does this check completely
resolve the leaked flags issue?
If load_misc_binary() leaks the have_execfd flag and the kernel falls back to
binfmt_script, it looks like exec_binprm() might unconditionally consume the
leaked flag and set bprm->executable to the script:
fs/exec.c:exec_binprm() {
...
if (unlikely(bprm->have_execfd)) {
if (bprm->executable) {
fput(exec);
return -ENOEXEC;
}
bprm->executable = exec;
} else
...
}
In the case of a setuid script with a statically linked interpreter (like
#!/sbin/busybox.static sh), would this cause begin_new_exec() to see a
non-NULL bprm->executable?
If so, would it bypass this cleanup and compute credentials from the script
rather than the interpreter, potentially honoring the setuid bit on the script?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-work-bpf-binfmt_misc-ptinterp-v1-0-ddb76c9a508e@kernel.org?part=1
next prev parent reply other threads:[~2026-07-20 10:00 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-20 9:33 [PATCH 00/21] binfmt_misc: transparent interpreters and PT_INTERP loader substitution Christian Brauner
2026-07-20 9:33 ` [PATCH 01/21] exec: do not act on a stale execfd request without an executable Christian Brauner
2026-07-20 10:00 ` sashiko-bot [this message]
2026-07-20 11:30 ` Christian Brauner
2026-07-20 12:44 ` Christian Brauner
2026-07-20 9:33 ` [PATCH 02/21] docs, binfmt_misc: keep general usage out of the handler sections Christian Brauner
2026-07-20 9:33 ` [PATCH 03/21] binfmt_misc: table-drive the register string flags Christian Brauner
2026-07-20 9:33 ` [PATCH 04/21] binfmt_misc: normalize the per-exec invocation flags Christian Brauner
2026-07-20 9:33 ` [PATCH 05/21] binfmt_misc: split out entry_open_interpreter() Christian Brauner
2026-07-20 9:33 ` [PATCH 06/21] binfmt_misc: split out build_interp_argv() Christian Brauner
2026-07-20 9:33 ` [PATCH 07/21] exec: release the replaced file with do_close_execat() Christian Brauner
2026-07-20 9:33 ` [PATCH 08/21] exec: add AT_FLAGS_TRANSPARENT_INTERP Christian Brauner
2026-07-20 9:33 ` [PATCH 09/21] exec: label mm->exe_file with the binary for a transparent dispatch Christian Brauner
2026-07-20 9:33 ` [PATCH 10/21] binfmt_misc: add transparent interpreter dispatch Christian Brauner
2026-07-20 9:33 ` [PATCH 11/21] binfmt_misc: add a static transparent flag 'T' Christian Brauner
2026-07-20 9:33 ` [PATCH 12/21] binfmt_misc: let a bpf handler run the interpreter transparently Christian Brauner
2026-07-20 9:33 ` [PATCH 13/21] selftests/exec: convert the binfmt_misc bpf test to the kselftest harness Christian Brauner
2026-07-20 9:33 ` [PATCH 14/21] selftests/exec: test the transparent binfmt_misc mode Christian Brauner
2026-07-20 11:44 ` sashiko-bot
2026-07-20 12:17 ` Christian Brauner
2026-07-20 9:33 ` [PATCH 15/21] binfmt_misc: document the transparent identity contract Christian Brauner
2026-07-20 9:33 ` [PATCH 16/21] exec: carry a PT_INTERP substitute in struct linux_binprm Christian Brauner
2026-07-20 9:33 ` [PATCH 17/21] binfmt_elf: consume a stashed PT_INTERP substitute Christian Brauner
2026-07-20 9:33 ` [PATCH 18/21] binfmt_misc: add the 'L' loader substitution flag Christian Brauner
2026-07-20 9:33 ` [PATCH 19/21] binfmt_misc: let a bpf handler request loader substitution Christian Brauner
2026-07-20 9:33 ` [PATCH 20/21] selftests/exec: test binfmt_misc " Christian Brauner
2026-07-20 12:22 ` sashiko-bot
2026-07-20 9:33 ` [PATCH 21/21] binfmt_misc: document " 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=20260720100022.9212B1F00A3A@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox