[PATCH v2 bpf-next 1/2] bpf: annotate file argument as __nullable in bpf_lsm_mmap_file

[PATCH v2 bpf-next 1/2] bpf: annotate file argument as __nullable in bpf_lsm_mmap_file

[Matt Bobrowski]

As reported in [0], anonymous memory mappings are not backed by a
struct file instance. Consequently, the struct file pointer passed to
the security_mmap_file() LSM hook is NULL in such cases.

The BPF verifier is currently unaware of this, allowing BPF LSM
programs to dereference this struct file pointer without needing to
perform an explicit NULL check. This leads to potential NULL pointer
dereference and a kernel crash.

Add a strong override for bpf_lsm_mmap_file() which annotates the
struct file pointer parameter with the __nullable suffix. This
explicitly informs the BPF verifier that this pointer (PTR_MAYBE_NULL)
can be NULL, forcing BPF LSM programs to perform a check on it before
dereferencing it.

[0] https://lore.kernel.org/bpf/5e460d3c.4c3e9.19adde547d8.Coremail.kaiyanm@hust.edu.cn/

Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
Closes: https://lore.kernel.org/bpf/5e460d3c.4c3e9.19adde547d8.Coremail.kaiyanm@hust.edu.cn/
Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
v2:
 - Updated the comment against the new strong definition for
   bpf_lsm_mmap_file(), clarifying the need for using the __nullable
   suffix annotation against the struct file pointer parameter name.

 MAINTAINERS                |  1 +
 kernel/bpf/Makefile        | 12 +++++++++++-
 kernel/bpf/bpf_lsm.c       |  5 +++--
 kernel/bpf/bpf_lsm_proto.c | 19 +++++++++++++++++++
 4 files changed, 34 insertions(+), 3 deletions(-)
 create mode 100644 kernel/bpf/bpf_lsm_proto.c

diff --git a/MAINTAINERS b/MAINTAINERS
index e36689cd7cc7..c531fae0dc06 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4754,6 +4754,7 @@ S:	Maintained
 F:	Documentation/bpf/prog_lsm.rst
 F:	include/linux/bpf_lsm.h
 F:	kernel/bpf/bpf_lsm.c
+F:	kernel/bpf/bpf_lsm_proto.c
 F:	kernel/trace/bpf_trace.c
 F:	security/bpf/
 
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 232cbc97434d..79cf22860a99 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -42,7 +42,17 @@ endif
 ifeq ($(CONFIG_BPF_JIT),y)
 obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o
 obj-$(CONFIG_BPF_SYSCALL) += cpumask.o
-obj-${CONFIG_BPF_LSM} += bpf_lsm.o
+# bpf_lsm_proto.o must precede bpf_lsm.o. The current pahole logic
+# deduplicates function prototypes within
+# btf_encoder__add_saved_func() by keeping the first instance seen. We
+# need the function prototype(s) in bpf_lsm_proto.o to take precedence
+# over those within bpf_lsm.o. Having bpf_lsm_proto.o precede
+# bpf_lsm.o ensures its DWARF CU is processed early, forcing the
+# generated BTF to contain the overrides.
+#
+# Notably, this is a temporary workaround whilst the deduplication
+# semantics within pahole are revisited accordingly.
+obj-${CONFIG_BPF_LSM} += bpf_lsm_proto.o bpf_lsm.o
 endif
 ifneq ($(CONFIG_CRYPTO),)
 obj-$(CONFIG_BPF_SYSCALL) += crypto.o
diff --git a/kernel/bpf/bpf_lsm.c b/kernel/bpf/bpf_lsm.c
index 7cb6e8d4282c..0c4a0c8e6f70 100644
--- a/kernel/bpf/bpf_lsm.c
+++ b/kernel/bpf/bpf_lsm.c
@@ -18,10 +18,11 @@
 #include <linux/bpf-cgroup.h>
 
 /* For every LSM hook that allows attachment of BPF programs, declare a nop
- * function where a BPF program can be attached.
+ * function where a BPF program can be attached. Notably, we qualify each with
+ * weak linkage such that strong overrides can be implemented if need be.
  */
 #define LSM_HOOK(RET, DEFAULT, NAME, ...)	\
-noinline RET bpf_lsm_##NAME(__VA_ARGS__)	\
+__weak noinline RET bpf_lsm_##NAME(__VA_ARGS__)	\
 {						\
 	return DEFAULT;				\
 }
diff --git a/kernel/bpf/bpf_lsm_proto.c b/kernel/bpf/bpf_lsm_proto.c
new file mode 100644
index 000000000000..44a54fd8045e
--- /dev/null
+++ b/kernel/bpf/bpf_lsm_proto.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2025 Google LLC.
+ */
+
+#include <linux/fs.h>
+#include <linux/bpf_lsm.h>
+
+/*
+ * Strong definition of the mmap_file() BPF LSM hook. The __nullable suffix on
+ * the struct file pointer parameter name marks it as PTR_MAYBE_NULL. This
+ * explicitly enforces that BPF LSM programs check for NULL before attempting to
+ * dereference it.
+ */
+int bpf_lsm_mmap_file(struct file *file__nullable, unsigned long reqprot,
+		      unsigned long prot, unsigned long flags)
+{
+	return 0;
+}
-- 
2.52.0.313.g674ac2bdf7-goog

[PATCH v2 bpf-next 2/2] selftests/bpf: add test case for BPF LSM hook bpf_lsm_mmap_file

[Matt Bobrowski]

Add a trivial test case asserting that the BPF verifier enforces
PTR_MAYBE_NULL semantics on the struct file pointer argument of BPF
LSM hook bpf_lsm_mmap_file().

Dereferencing the struct file pointer passed into bpf_lsm_mmap_file()
without explicitly performing a NULL check first should not be
permitted by the BPF verifier as it can lead to NULL pointer
dereferences and a kernel crash.

Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
---
 .../selftests/bpf/progs/verifier_lsm.c        | 31 ++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_lsm.c b/tools/testing/selftests/bpf/progs/verifier_lsm.c
index 6af9100a37ff..38e8e9176862 100644
--- a/tools/testing/selftests/bpf/progs/verifier_lsm.c
+++ b/tools/testing/selftests/bpf/progs/verifier_lsm.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 
-#include <linux/bpf.h>
+#include <vmlinux.h>
 #include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
 #include "bpf_misc.h"
 
 SEC("lsm/file_permission")
@@ -159,4 +160,32 @@ __naked int disabled_hook_test3(void *ctx)
 	::: __clobber_all);
 }
 
+SEC("lsm/mmap_file")
+__description("not null checking nullable pointer in bpf_lsm_mmap_file")
+__failure __msg("R1 invalid mem access 'trusted_ptr_or_null_'")
+int BPF_PROG(no_null_check, struct file *file)
+{
+	struct inode *inode;
+
+	inode = file->f_inode;
+	__sink(inode);
+
+	return 0;
+}
+
+SEC("lsm/mmap_file")
+__description("null checking nullable pointer in bpf_lsm_mmap_file")
+__success
+int BPF_PROG(null_check, struct file *file)
+{
+	struct inode *inode;
+
+	if (file) {
+		inode = file->f_inode;
+		__sink(inode);
+	}
+
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.52.0.313.g674ac2bdf7-goog

Re: [PATCH v2 bpf-next 1/2] bpf: annotate file argument as __nullable in bpf_lsm_mmap_file

[Song Liu]

On Tue, Dec 16, 2025 at 5:30 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
[...]
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 232cbc97434d..79cf22860a99 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -42,7 +42,17 @@ endif
>  ifeq ($(CONFIG_BPF_JIT),y)
>  obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o
>  obj-$(CONFIG_BPF_SYSCALL) += cpumask.o
> -obj-${CONFIG_BPF_LSM} += bpf_lsm.o
> +# bpf_lsm_proto.o must precede bpf_lsm.o. The current pahole logic
> +# deduplicates function prototypes within
> +# btf_encoder__add_saved_func() by keeping the first instance seen. We
> +# need the function prototype(s) in bpf_lsm_proto.o to take precedence
> +# over those within bpf_lsm.o. Having bpf_lsm_proto.o precede
> +# bpf_lsm.o ensures its DWARF CU is processed early, forcing the
> +# generated BTF to contain the overrides.
> +#
> +# Notably, this is a temporary workaround whilst the deduplication
> +# semantics within pahole are revisited accordingly.

This is quite tricky, but I can confirm we need bpf_lsm_proto.o first.

Acked-by: Song Liu <song@kernel.org>

> +obj-${CONFIG_BPF_LSM} += bpf_lsm_proto.o bpf_lsm.o
>  endif
>  ifneq ($(CONFIG_CRYPTO),)
>  obj-$(CONFIG_BPF_SYSCALL) += crypto.o
[...]

Re: [PATCH v2 bpf-next 2/2] selftests/bpf: add test case for BPF LSM hook bpf_lsm_mmap_file

[Song Liu]

On Tue, Dec 16, 2025 at 5:30 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> Add a trivial test case asserting that the BPF verifier enforces
> PTR_MAYBE_NULL semantics on the struct file pointer argument of BPF
> LSM hook bpf_lsm_mmap_file().
>
> Dereferencing the struct file pointer passed into bpf_lsm_mmap_file()
> without explicitly performing a NULL check first should not be
> permitted by the BPF verifier as it can lead to NULL pointer
> dereferences and a kernel crash.
>
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>

Acked-by: Song Liu <song@kernel.org>

Re: [PATCH v2 bpf-next 1/2] bpf: annotate file argument as __nullable in bpf_lsm_mmap_file

[Matt Bobrowski]

On Wed, Dec 17, 2025 at 04:45:34AM +0900, Song Liu wrote:
> On Tue, Dec 16, 2025 at 5:30 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
> [...]
> > diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> > index 232cbc97434d..79cf22860a99 100644
> > --- a/kernel/bpf/Makefile
> > +++ b/kernel/bpf/Makefile
> > @@ -42,7 +42,17 @@ endif
> >  ifeq ($(CONFIG_BPF_JIT),y)
> >  obj-$(CONFIG_BPF_SYSCALL) += bpf_struct_ops.o
> >  obj-$(CONFIG_BPF_SYSCALL) += cpumask.o
> > -obj-${CONFIG_BPF_LSM} += bpf_lsm.o
> > +# bpf_lsm_proto.o must precede bpf_lsm.o. The current pahole logic
> > +# deduplicates function prototypes within
> > +# btf_encoder__add_saved_func() by keeping the first instance seen. We
> > +# need the function prototype(s) in bpf_lsm_proto.o to take precedence
> > +# over those within bpf_lsm.o. Having bpf_lsm_proto.o precede
> > +# bpf_lsm.o ensures its DWARF CU is processed early, forcing the
> > +# generated BTF to contain the overrides.
> > +#
> > +# Notably, this is a temporary workaround whilst the deduplication
> > +# semantics within pahole are revisited accordingly.
> 
> This is quite tricky, but I can confirm we need bpf_lsm_proto.o first.

Yes, agree, but it's an outright "hack" at this point. Note that I'm
also going to send a fix addressing a shortcoming within pahole, as
per this thread [0]. I'm just waiting to see what the BTF experts have
to say about it.

[0] https://lore.kernel.org/bpf/aTlFKI2IeHQ2-TSE@google.com/

Re: [PATCH v2 bpf-next 1/2] bpf: annotate file argument as __nullable in bpf_lsm_mmap_file

[Alexei Starovoitov]

On Tue, Dec 16, 2025 at 3:30 AM Matt Bobrowski <mattbobrowski@google.com> wrote:
>
> As reported in [0], anonymous memory mappings are not backed by a
> struct file instance. Consequently, the struct file pointer passed to
> the security_mmap_file() LSM hook is NULL in such cases.
>
> The BPF verifier is currently unaware of this, allowing BPF LSM
> programs to dereference this struct file pointer without needing to
> perform an explicit NULL check. This leads to potential NULL pointer
> dereference and a kernel crash.
>
> Add a strong override for bpf_lsm_mmap_file() which annotates the
> struct file pointer parameter with the __nullable suffix. This
> explicitly informs the BPF verifier that this pointer (PTR_MAYBE_NULL)
> can be NULL, forcing BPF LSM programs to perform a check on it before
> dereferencing it.
>
> [0] https://lore.kernel.org/bpf/5e460d3c.4c3e9.19adde547d8.Coremail.kaiyanm@hust.edu.cn/
>
> Reported-by: Kaiyan Mei <M202472210@hust.edu.cn>
> Reported-by: Yinhao Hu <dddddd@hust.edu.cn>
> Reviewed-by: Dongliang Mu <dzm91@hust.edu.cn>
> Closes: https://lore.kernel.org/bpf/5e460d3c.4c3e9.19adde547d8.Coremail.kaiyanm@hust.edu.cn/
> Signed-off-by: Matt Bobrowski <mattbobrowski@google.com>
> ---
> v2:
>  - Updated the comment against the new strong definition for
>    bpf_lsm_mmap_file(), clarifying the need for using the __nullable
>    suffix annotation against the struct file pointer parameter name.

It was applied.
pw-bot is asleep.