Re: bpf: mmap_file LSM hook allows NULL pointer dereference

[Matt Bobrowski <mattbobrowski@google.com>]

On Wed, Dec 03, 2025 at 10:23:43AM -0800, Alexei Starovoitov wrote:
> On Wed, Dec 3, 2025 at 12:47 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > > We can play tricks with __weak. Like:
> > >
> > > diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
> > > index 7cb6e8d4282c..60d269a85bf1 100644
> > > --- a/kernel/bpf/bpf_lsm.c
> > > +++ b/kernel/bpf/bpf_lsm.c
> > > @@ -21,7 +21,7 @@
> > >   * function where a BPF program can be attached.
> > >   */
> > >  #define LSM_HOOK(RET, DEFAULT, NAME, ...)      \
> > > -noinline RET bpf_lsm_##NAME(__VA_ARGS__)       \
> > > +__weak noinline RET bpf_lsm_##NAME(__VA_ARGS__)        \
> > >
> > > diff kernel/bpf/bpf_lsm_proto.c
> > >
> > > +int bpf_lsm_mmap_file(struct file *file__nullable, unsigned long reqprot,
> > > +                     unsigned long prot, unsigned long flags)
> > > +{
> > > +       return 0;
> > > +}
> > >
> > > and above one with __nullable will be in vmlinux BTF.
> > >
> > > afaik __weak functions are not removed by linker when in non-LTO,
> > > but it's still better than
> > > +#define bpf_lsm_mmap_file bpf_lsm_mmap_file__original
> > > No need to change bpf_lsm.h either.
> >
> > Annotating with a weak attribute would be quite nice, but the compiler
> > will complain about the redefinition of the symbol
> > bpf_lsm_mmap_file. To avoid this, we'd still need to rely on the
> > rename and ignore dance by using the aforementioned define, which at
> > that point would still result in both symbols being exposed in both
> > BTF and the .text section.
> 
> Not quite. You missed this part in the above:
> 
> > > diff kernel/bpf/bpf_lsm_proto.c
> 
> it's a different file.

Yes, yes, this will work. However, as discussed, it's fundamentally
reliant on a small "hack" which I've implemented within
kernel/bpf/Makefile here [0] to workaround current pahole
deduplication logic.

Andrii and Eduard,

I’d like your input on a pahole BTF generation issue which I've
recently come across. In the series I just sent [0], I had to
implement a workaround to force pahole to process bpf_lsm_proto.o
before bpf_lsm.o.

This was necessary to ensure pahole generates BTF for the strong
definition of bpf_lsm_mmap_file() (in bpf_lsm_proto.c) rather than the
weak definition (in bpf_lsm.c). Without this forced ordering, pahole
processed the weak definition first, resulting in a state array like
this:

```
btf_encoder.func_states.array[N] = bpf_lsm_mmap_file (weak
definition from bpf_lsm.o)

btf_encoder.func_states.array[N+1] = bpf_lsm_mmap_file (strong
definition from bpf_lsm_proto.o)
```

Because the deduplication logic in btf_encoder__add_saved_funcs()
folds duplicates (those determined by saved_functions_combine()) into
the first occurrence, the resulting BTF was derived from the weak
definition. This is incorrect, as the strong definition is the one
actually linked into the final vmlinux image.

An obvious fix that immediately came to mind here was to essentially
teach pahole about strong function prototype definitions, and prefer
to emit BTF for those instead of any weak defined counterparts?

[0] https://lore.kernel.org/bpf/20251210090701.2753545-1-mattbobrowski@google.com/T/#me14d534fb559a349c46e094f18c63d477644d511