Re: bpf: mmap_file LSM hook allows NULL pointer dereference

[Matt Bobrowski <mattbobrowski@google.com>]

On Tue, Dec 02, 2025 at 09:27:17AM -0800, Alexei Starovoitov wrote:
> On Tue, Dec 2, 2025 at 6:55 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> >
> > On Tue, Dec 02, 2025 at 10:38:55AM +0000, Matt Bobrowski wrote:
> > > On Tue, Dec 02, 2025 at 03:09:42PM +0800, 梅开彦 wrote:
> > > > Our fuzzer tool discovered a NULL pointer dereference vulnerability in the BPF subsystem. The vulnerability is triggered when a BPF LSM program, attached to the `bpf_lsm_mmap_file`, receives a NULL pointer for the `struct file *` argument during an anonymous memory mapping operation but attempts to dereference it without a NULL check.
> > > >
> > > > Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> > > > Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> > > > Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
> > > >
> > > > ## Root Cause
> > > >
> > > > The crash is caused by a NULL pointer dereference within an eBPF program attached to the `bpf_lsm_mmap_file` LSM hook. The trigger occurs when a user calls the `mmap` syscall with the `MAP_ANONYMOUS` flag. This action creates a file-less memory mapping, causing the kernel to invoke the `security_mmap_file` hook with a NULL `struct file *` argument. If the attached BPF program assumes this pointer is always valid and attempts to dereference it without a NULL check, it immediately causes a page fault, leading to a kernel panic.
> > >
> > > Thanks for the report. Can confirm, and a more simplified reproducer
> > > can be found below:
> > >
> > > BPF:
> > > ```
> > > SEC("lsm.s/mmap_file")
> > > int BPF_PROG(mmap_file, struct file *file)
> > > {
> > >         bpf_printk("i_ino=%llu", file->f_inode->i_ino);
> > >         return 0;
> > > }
> > > ```
> > >
> > > Stimulus:
> > > ```
> > > #include <stdio.h>
> > > #include <string.h>
> > > #include <sys/mman.h>
> > >
> > > int main(int argc, char** argv) {
> > >   void* p;
> > >
> > >   p = mmap(NULL, 4096, PROT_READ | PROT_WRITE | PROT_EXEC,
> > >            MAP_FIXED | MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > >   if (p == MAP_FAILED) {
> > >     perror("mmap");
> > >     return 1;
> > >   }
> > >
> > >   memset(p, 0, 4096);
> > >   munmap(p, 4096);
> > >   return 0;
> > > }
> > > ```
> > >
> > > My initial thought here is that bpf_lsm_is_trusted() is treating
> > > pointer arguments provided to security_mmap_file() simply as
> > > PTR_TRUSTED, when in reality it should be something like PTR_TRUSTED |
> > > PTR_MAYBE_NULL instead. Anyway, I can take a look and send through an
> > > appropriate fix.
> >
> > So, thinking about this a little I think we have a couple options when
> > it comes to addressing this. We can either:
> >
> > a) Add bpf_lsm_mmap_file() to the untrusted_lsm_hooks set. This will
> > effectively mark the struct file pointer argument passed into
> > bpf_lsm_mmap_file() as being untrusted (i.e. the register will not
> > carry a PTR_TRUSTED type). The caveat with this approach however is
> > that it'll have negative effects (i.e. cannot be supplied to BPF
> > kfuncs that accept KF_TRUSTED_ARGS arguments) on those struct file
> > pointer arguments that are considered to be valid and trusted.
> >
> > b) Update the generic LSM_HOOK() declaration for mmap_file
> > (security_mmap_file()) within lsm_hook_defs.h such that the struct
> > file pointer argument name contains a "__nullable" suffix. This will
> > allow btf_ctx_access() to apply the PTR_MAYBE_NULL type onto the
> > backing register holding a reference to the struct file pointer,
> > ultimately forcing the BPF program to perform a NULL check before
> > attempting to dereference it. The caveat with this approach is that
> > BPF program verification, and ultimately its runtime safety
> > guarantees, would be tied in with generic LSM infrastructure which I
> > don't think is a good idea. Subtle breakage could occur literally at
> > any point.
> >
> > c) Provide an override declaration/definition for bpf_lsm_mmap_file()
> > within include/linux/bpf_lsm.h and kernel/bpf/bpf_lsm.c such that the
> > struct file pointer argument is named file__nullable instead. Similar
> 
> I don't yet see how you can accomplish this, but it's indeed the best
> option.
> There is already:
> FUNC 'bpf_lsm_mmap_file' type_id=...
> Magic hack to
> #define LSM_HOOK(RET, DEFAULT, NAME, ...)       \
> noinline RET bpf_lsm_##NAME(__VA_ARGS__)        \
> ?

Hm, I too was questioning whether this was initially even possible,
but I managed to get it working by doing the following (perhaps this
is also what you had meant when you mentioned using the distinct
suffix below):

```
diff --git a/include/linux/bpf_lsm.h b/include/linux/bpf_lsm.h
index 643809cc78c3..ddb328ba856d 100644
--- a/include/linux/bpf_lsm.h
+++ b/include/linux/bpf_lsm.h
@@ -14,11 +14,18 @@

 #ifdef CONFIG_BPF_LSM

+int bpf_lsm_mmap_file(struct file *file__nullable, unsigned long reqprot,
+                     unsigned long prot, unsigned long flags);
+
+#define bpf_lsm_mmap_file bpf_lsm_mmap_file__original
+
 #define LSM_HOOK(RET, DEFAULT, NAME, ...) \
        RET bpf_lsm_##NAME(__VA_ARGS__);
 #include <linux/lsm_hook_defs.h>
 #undef LSM_HOOK

+#undef bpf_lsm_mmap_file
+
 struct bpf_storage_blob {
        struct bpf_local_storage __rcu *storage;
 };
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 7cb6e8d4282c..48259738e032 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -20,6 +20,14 @@
 /* For every LSM hook that allows attachment of BPF programs, declare a nop
  * function where a BPF program can be attached.
  */
+noinline int bpf_lsm_mmap_file(struct file *file__nullable, unsigned long reqprot,
+                             unsigned long prot, unsigned long flags)
+{
+       return 0;
+}
+
+#define bpf_lsm_mmap_file bpf_lsm_mmap_file__original
+
 #define LSM_HOOK(RET, DEFAULT, NAME, ...)      \
 noinline RET bpf_lsm_##NAME(__VA_ARGS__)       \
 {                                              \
@@ -29,6 +37,8 @@ noinline RET bpf_lsm_##NAME(__VA_ARGS__)      \
 #include <linux/lsm_hook_defs.h>
 #undef LSM_HOOK

+#undef bpf_lsm_mmap_file
+
 #define LSM_HOOK(RET, DEFAULT, NAME, ...) BTF_ID(func, bpf_lsm_##NAME)
 BTF_SET_START(bpf_lsm_hooks)
 #include <linux/lsm_hook_defs.h>
```

> or a flavor named with a distinct suffix ?
> bpf_lsm_mmap_file___suffix(struct file *file__nullable, ...) ?
> 
> To address the same issue with tracepoints we went with
> raw_tp_null_args[].
> It's ugly, but gets the job done. We can probably apply it
> to lsm args too.

Unfortunately, I'm not overly familiar with the whole
raw_tp_null_args[] thing, so I don't really know how this works. I'll
need to take a look.

> tbh option (b) could have been a choice or even:
> -int security_mmap_file(struct file *file, unsigned long prot,
> +int security_mmap_file(struct file *file__nullable, unsigned long prot,
> 
> but it's an operational headache due to different subsystems/maintainers.

Agree, hence why I think we should just add the necessary workaround
directly within the BPF LSM.

> Long term the plan is to wait for gcc/pahole to fully support
> btf_decl_tags and switch to that.

Yes, I literally hit send on my previous email only for this exact
option to also pop into my head. But as I read more into it on my
commute home, I gathered that there was some inherent dependency on
the BPF toolchain which probably isn't ideal at this point either.

Another alternative that has literally just come to mind is the
possibility of introducing another BTF set (i.e. nullable_args). A
function (LSM hook) associated with this new BTF set will basically
force the BPF verifier to strictly treat all of its associated pointer
type arguments as nullable. Therefore, resulting in the PTR_MAYBE_NULL
type being applied to any register that may be referencing one of
these function pointer type arguments. This kind of check against this
new BTF set could be performed alongside the btf_param_match_suffix()
check within btf_ctx_access(). What do you think?