Re: [BUG] bpf, lsm: new first argument in 7.4 inode hooks silently breaks BPF LSMs

[roblabla <linux@roblab.la>]

Hi,

Thanks for the replies!

> LSMs that need to provide a stable API must provide an abstraction
> layer between the LSM hook callbacks and their stable API.  The LSM
> framework is not responsible for the stability of individual LSM APIs.

Understood, that lines up with my expectations. I think the biggest
thing here would not necessarily be stability guarantees, but rather to
make sure that the eBPFs fail to load when the compatibility is broken.
But that's probably a problem for the eBPF side, and not LSM, I assume.

> So, in short, would it be possible to send a follow up to Daan's
> commit and move `struct mnt_idmap *idmap` to the end of LSM argument
> list?

If this is possible, that'd be nice so that we avoid this breakage in
the short term, since the change isn't in a released kernel yet.

> Having said that, there is a way to handle this, although ugly and
> *everyone* using specific LSMs would need to do this on their own.
> Instead of using convenience BPF_PROG() macro to define BPF program,
> you just use raw `u64 ctx[]` access, and then cast ctx[0] to either
> `struct mnt_idmap *` on new kernel, or `struct dentry *` on old
> kernel.

So, we already do similar tricks in the software we ship (which uses
various eBPF program types, not just LSM) to cover kernels 4.11 to 7.3,
using a mixture of CO-RE tricks similar to what you describe and
per-kernel builds selected at runtime.

Here's the problem though: We ship to customers, and we don't control
when they upgrade their kernel relative to our agent. When the kernel
changes a hook signature, a program built before we could handle that
new version still loads fine and just reads the wrong arguments, which
causes some really gnarly bugs that can cause really bad breakage (such
as accidentally denying events that should have been allowed, often
resulting in machines left broken).

So what would be really nice is a way to make eBPF code robust against
kernel changes such that it either works, or fails to load, but doesn't
do the halfway "loads but silently misbehaves". Here are two ideas that
have been in the back of my mind that could help solve this issue (though
I understand that they may be rather large changes, I'm putting them out
there mostly as a conversation starter):

1. Could libbpf/the kernel have a mismatch check on hook arguments? I
   understand that libbpf and the verifier don't know the types a
   program expects, since BPF_PROG() only casts u64s. But the macro
   does know the types at compile time. Could it (or an opt-in variant)
   record the expected prototype in the program's BTF, so that libbpf
   can compare it against the LSM hook's prototype in vmlinux BTF and
   refuse to load on a mismatch?

2. Since kernel BTF has parameter names for the bpf_lsm_* functions,
   could we maybe have CO-RE-style relocation to resolve "the argument
   named dentry" to the argument index at load time? This way, adding a
   new argument could happen anywhere in the argument list without
   breaking eBPFs, and removing an argument will (correctly) cause our
   eBPF to stop loading. It does mean the eBPF programs must provide the
   same argument names, but maybe that's an acceptable tradeoff?

Thanks,
roblabla