Linux-mm Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: "Farid Zakaria" <farid.m.zakaria@gmail.com>
To: "Christian Brauner" <brauner@kernel.org>,
	"Farid Zakaria" <farid.m.zakaria@gmail.com>
Cc: "Alexei Starovoitov" <ast@kernel.org>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Martin KaFai Lau" <martin.lau@linux.dev>,
	"Shuah Khan" <shuah@kernel.org>,
	"Andrii Nakryiko" <andrii@kernel.org>,
	"Kees Cook" <kees@kernel.org>,
	"Alexander Viro" <viro@zeniv.linux.org.uk>,
	"Jan Kara" <jack@suse.cz>, "Jonathan Corbet" <corbet@lwn.net>,
	"Jann Horn" <jannh@google.com>,
	"John Ericson" <mail@johnericson.me>,
	<linux-fsdevel@vger.kernel.org>, <linux-mm@kvack.org>,
	<linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>,
	<linux-doc@vger.kernel.org>, <linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v2 0/5] binfmt_misc: bpf-backed binary type handlers
Date: Sun, 12 Jul 2026 20:29:27 -0700	[thread overview]
Message-ID: <DJX4IFO55D9S.3IV3K5XICJAEV@gmail.com> (raw)
In-Reply-To: <20260712-abluft-brutkasten-aufladen-c8063a4a1a0a@brauner>

On Sun Jul 12, 2026 at 5:32 AM PDT, Christian Brauner wrote:
>> This is a continuation of Christian's bpf-backed binfmt_misc POC [1], which he offered
>> to hand off to me. I am carrying it forward. The kernel design is his and is
>> unchanged. This series rebases it onto his binfmt_misc locking/cleanup series [2]
>> and adds selftests (+ fixed from the one's shared), and therefore does
>> not apply to mainline alone.
>> 
>> As for motivation for this whole change, Christian did a great VL;MR;
>> write-up [1].
>> 
>> TL;DR: binfmt_misc can match a binary and hand it to a fixed interpreter,
>> but it can't match programmatically or compute the interpreter per binary.
>> The Nix community would love to support relocatable binaries, where the
>> correct loader can only be found relative to the binary itself.
>> This adds a 'B' handler type: a binfmt_misc_ops struct_ops program that
>> inspects the binary and picks the interpreter with one new kfunc,
>> bpf_binprm_set_interp().
>> 
>>   bpftool struct_ops register nix_origin.bpf.o /sys/fs/bpf
>>   echo ':nix-origin:B:nix_origin::::' > /proc/sys/fs/binfmt_misc/register
>
> Thanks! So I've spent some more time working on this because I had some
> future ideas that I needed to make sure could be accomodated by this
> (currently on vacation but hey...). So the version I have givex this a
> better design and has future extensibility in mind:
>

I also enjoy hacking over my vacation. I'm often told to take break but
some of the best ideas come when on vacation :)

> - The single sleepable load op is split into two ops:
>
>       struct binfmt_misc_ops {
>       	bool (*match)(struct linux_binprm *bprm);
>       	int (*load)(struct linux_binprm *bprm);
>       	char name[BINFMT_MISC_OPS_NAME_MAX];
>       };
>
>   The non-sleepable match program runs from the RCU entry lookup
>   walk itself, exactly like magic and extension matching: same
>   registration order, same first-match-wins semantics, and it can
>   only rely on the prefetched bprm->buf. It must be free of side
>   effects since the walk may be restarted.
>

Is relying on brpm->buf enough?
Right now that's only 256 bytes I think, which is not enough to read
segments we might care about. That worked fine when it was just a magic
number but the idea with the eBPF program is to make decisions based on
more data.

In the selftest I provided, the `PT_INTERP` segment is already at file
offset 0x318 (792). I was imaginging NixOS having to support
`PT_INTERP_NIX` in order for the produced binaries to be backwards
compatible with older kernels.

The other idea would be to make the `match()` broad and select
everything but then nearly all ELF64 binaries would match and then pay
the price to `load()` and ultimately `-ENOEXEC`. Seems like it would
make multiple BPF binfmt programs less useful.


>   The sleepable load program runs once the walk has committed to the
>   entry and does the file reading and the interpreter selection. Both
>   ops are mandatory, the struct_ops plumbing enforces the sleepability
>   of each, and since bpf_binprm_set_interp() is KF_SLEEPABLE a match
>   program cannot select an interpreter by construction.
>
> - A match is a commitment. A failing load program fails the exec instead
>   of falling through to later entries. -ENOEXEC keeps its usual meaning
>   and hands the binary to the remaining binary formats, for a handler
>   that discovers it cannot serve the binary after all.
>
>   This kills the part of v1 I disliked the most: the skip cursor and the
>   leave-and-rescan loop in load_misc_binary() are gone. The walk is
>   never left and re-entered, and 'B' entries need no special semantics
>   against concurrent (un)registration anymore.
>
>   Your v2 changelog note about keeping the bpf retry loop in the
>   __free() style is moot as a consequence. The loop no longer exists.
>
> - The load return convention flipped: 0 now means success after the
>   program called bpf_binprm_set_interp(). Returning 0 without having
>   selected an interpreter or returning a positive value is treated as
>   -ENOEXEC, other negative errnos fail the exec.
>
>   So the "return bpf_binprm_set_interp(...) ?: 1" idiom from the v1-era
>   programs becomes plain "return bpf_binprm_set_interp(...)".
>
> - The handler name moved from the offset field to the interpreter field
>   that field consistently names whoever supplies the interpreter, a path
>   for static entries, a handler for 'B' entries. Offset, magic, and mask
>   must be empty:
>
> 	echo ':nix-origin:B::::nix_origin:' > register
>
> - 'C' is allowed now, v1 rejected it. It behaves exactly as for a static
>    entry. The setuid transition stays gated by vfsuid_has_mapping() in
>    the caller's user namespace. Which makes 'B' handlers usable for a
>    per-binary loader over setuid binaries. 'F' stays rejected as there
>    is no fixed interpreter to pre-open (I have other ideas how we'll do
>    something like it later.).
>
> Nothing changed in the exec patch, the fs kfuncs patch, the kfunc
> itself, or the registry/namespacing model.
>
> I can send v3 in a bit if that's ok.

I don't mind at all you sending v3 and in fact I've been enjoying your
involvement. I didn't know what to expect when I offered this idea up to
the community. 

I'm happy to keep co-developing this with you within a design you feel
acceptable with. Please let me know how I can remain engaged and
helpful.

One last thing I was thinking about is that we will also need to support
$ORIGIN in the shebang path, however I just tested it and this current broad
BPF solution can largely handle it [1], with a small wrinkle.

  $ printf '#!$ORIGIN/interp\n' > /opt/app/greet
  $ cp ./interp /opt/app/interp     # any interpreter/loader
  $ chmod +x /opt/app/greet /opt/app/interp

  # stock kernel: binfmt_script opens the literal "$ORIGIN/interp"
  $ /opt/app/greet
  bash: /opt/app/greet: $ORIGIN/interp: bad interpreter: No such file or directory

  # with the handler registered, $ORIGIN resolves to the script's dir
  $ bpftool struct_ops register shebang_origin.bpf.o /sys/fs/bpf
  $ echo ':shebang-origin:B:shebang_origin::::' > /proc/sys/fs/binfmt_misc/register
  $ /opt/app/greet
  <runs /opt/app/interp, the loader found next to the script>

The wrinkle is that it can't express today is the single optional argument.
(i.e. `#!interp arg" -> argv[1]=arg`). We might ned a way to express
that in the load.

Anywyas, thanks again. All your ideas made sense modulo I'm unsure if
`bprm->buf` is enough to make a decision...

[1] https://gist.github.com/fzakaria/2e1e1c44fa488a951674f8761c672366



      reply	other threads:[~2026-07-13  3:29 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-12  4:08 [PATCH v2 0/5] binfmt_misc: bpf-backed binary type handlers Farid Zakaria
2026-07-12  4:08 ` [PATCH v2 1/5] exec: stash a bpf-selected interpreter in struct linux_binprm Farid Zakaria
2026-07-12  4:08 ` [PATCH v2 2/5] binfmt_misc: add binfmt_misc_ops bpf struct_ops Farid Zakaria
2026-07-12  4:08 ` [PATCH v2 3/5] binfmt_misc: wire up bpf-backed 'B' entries Farid Zakaria
2026-07-12  4:08 ` [PATCH v2 4/5] bpf: allow fs kfuncs for binfmt_misc_ops programs Farid Zakaria
2026-07-12  4:08 ` [PATCH v2 5/5] selftests/exec: add binfmt_misc bpf-backed handler test Farid Zakaria
2026-07-12 12:32 ` [PATCH v2 0/5] binfmt_misc: bpf-backed binary type handlers Christian Brauner
2026-07-13  3:29   ` Farid Zakaria [this message]

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=DJX4IFO55D9S.3IV3K5XICJAEV@gmail.com \
    --to=farid.m.zakaria@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brauner@kernel.org \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=jack@suse.cz \
    --cc=jannh@google.com \
    --cc=kees@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mail@johnericson.me \
    --cc=martin.lau@linux.dev \
    --cc=shuah@kernel.org \
    --cc=viro@zeniv.linux.org.uk \
    /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