Re: [PATCH bpf-next v2 1/9] bpf: Support __arena and __arena_nullable kfunc argument suffixes
"Kumar Kartikeya Dwivedi" <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On Sun Jul 26, 2026 at 3:47 AM CEST, sashiko-bot wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [High] The `__arena_nullable` kfunc argument design uses a 32-bit truncated value of 0 as a sentinel for NULL, which collides with valid memory allocations from `bpf_arena_alloc_pages()`. > -- > > --- Patch [1]: [PATCH bpf-next v2 1/9] bpf: Support __arena and __arena_nullable kfunc argument suffixes --- > commit fb759ca6f84030ccde9ee83f1e83634342bdae0d > Author: Tejun Heo <[email protected]> > > bpf: Support __arena and __arena_nullable kfunc argument suffixes > > This patch adds support for passing BPF arena pointers to kfuncs by introducing > __arena and __arena_nullable argument suffixes. The verifier processes these > suffixes and allows JITs to handle the translation (rebasing) at the call site. > __arena rebases unconditionally, while __arena_nullable preserves NULL by skipping > the rebase when the arena offset is zero. > >> diff --git a/Documentation/bpf/kfuncs.rst b/Documentation/bpf/kfuncs.rst >> index c801a330aece0..9809680bee43e 100644 >> --- a/Documentation/bpf/kfuncs.rst >> +++ b/Documentation/bpf/kfuncs.rst > [ ... ] >> @@ -250,6 +250,33 @@ Or:: >> ... >> } >> >> +2.3.7 __arena and __arena_nullable Annotations >> +---------------------------------------------- >> + >> +Both annotations indicate that the pointer argument points into the >> +calling program's arena. The JIT rebases the value at the call site so >> +the kfunc receives a directly dereferenceable kernel address, subject to >> +the access rules described in :ref:`BPF_kfunc_arena_access` (at most >> +``GUARD_SZ / 2``, 32 KiB, past the pointer in a single unchecked access). >> + >> +With ``__arena`` the rebase is unconditional and the argument is never >> +NULL: a value whose lower 32 bits are zero arrives as the arena base >> +address (arena offset 0). The kfunc must not check the argument for NULL. >> +With ``__arena_nullable`` such a value arrives as NULL instead and the >> +kfunc must check before dereferencing. > > [Severity: High] > Does this __arena_nullable design cause a collision with valid allocations? > > If an arena is mapped at a 4GB-aligned boundary, the user_vm_start address will > have its lower 32 bits as 0. When bpf_arena_alloc_pages() returns the first > free page at offset 0, the resulting user pointer also has its lower 32 bits > equal to 0. > > If a BPF program passes this valid pointer to an __arena_nullable kfunc, won't > the JIT see the 0 value in the lower 32 bits and incorrectly pass NULL to the > kfunc instead of the valid arena base address? Yes, but it is intentional. Referencing the first page is equivalent to NULL, or start of the arena "address space". > >> + >> +An example is given below:: > [ ... ] >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 641c3c62c1ecf..f3e6190f639e4 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c > [ ... ] >> @@ -12056,6 +12066,35 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_call_arg_me >> >> t = btf_type_skip_modifiers(btf, args[i].type, NULL); >> >> + if (is_kfunc_arg_arena(btf, &args[i]) || is_kfunc_arg_arena_nullable(btf, &args[i])) { > > [Severity: High] > Should check_kfunc_args() or the arena allocator ensure that offset 0 is > reserved? > > If offset 0 is not reserved, the verifier will allow programs to pass a valid > offset 0 allocation to __arena_nullable arguments. The JIT will then silently > translate this to NULL, causing unexpected behavior in the kfunc since it will > assume the optional argument was not provided. It will always be left unmapped in practice. > >> + if (verifier_bug_if(!btf_type_is_ptr(t), env, >> + "kfunc %s arg#%d has __arena tag on non-pointer", >> + func_name, i)) >> + return -EFAULT;