> > - 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.

Ok, so your idea is to have multiple bpf programs that look for
different interpreters. Yeah, then you have to be able to sleep because
you need to be able to fault. It makes the code uglier but I can see how
that's useful. I'll see how nice I can make that.

> >   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.

Ah, fun. binfmt_misc wasn't able to express this at all. It's good to
close that gap. This can just be done by adding
bpf_binprm_set_interp_arg().


Reply via email to