Re: bpf: mmap_file LSM hook allows NULL pointer dereference

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

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.