Re: [PATCH bpf 1/2] bpf: Avoid eliding lookup NULLness for inner map templates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The patch relies on the `BPF_F_INNER_MAP` flag to prevent unsafe NULL-check elision for inner map templates. However, this fix is fundamentally incomplete because `BPF_MAP_TYPE_PERCPU_ARRAY` explicitly rejects the `BPF_F_INNER_MAP` flag during creation. This leaves per-CPU arrays vulnerable to out-of-bounds memory accesses and NULL pointer dereferences.
--

commit 3ab30507af620d9da556d3b2d80a1f172474785b
Author: Sun Jian <sun.jian.kdev@gmail.com>

bpf: Avoid eliding lookup NULLness for inner map templates

This commit prevents the verifier from eliding NULLness checks for array maps
used as inner map templates. It achieves this by requiring that the
BPF_F_INNER_MAP flag is not set on the map before clearing the PTR_MAYBE_NULL
flag from the register.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 69d75515ed3f7..a32704bb5e058 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -10577,6 +10577,7 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  
>  		if (func_id == BPF_FUNC_map_lookup_elem &&
>  		    can_elide_value_nullness(meta.map.ptr->map_type) &&
> +		    !(meta.map.ptr->map_flags & BPF_F_INNER_MAP) &&

[Severity: Critical]
This isn't a bug introduced by this patch, but does this check completely
prevent NULL-check elision for all inner map templates?

Looking at can_elide_value_nullness(), it allows both BPF_MAP_TYPE_ARRAY
and BPF_MAP_TYPE_PERCPU_ARRAY.

However, in array_map_alloc_check(), the BPF_F_INNER_MAP flag is
explicitly rejected for per-CPU arrays:

kernel/bpf/arraymap.c:array_map_alloc_check() {
    if (attr->map_type != BPF_MAP_TYPE_ARRAY &&
        attr->map_flags & (BPF_F_MMAPABLE | BPF_F_INNER_MAP))
        return -EINVAL;
    ...
}

Since BPF_MAP_TYPE_PERCPU_ARRAY can never have the BPF_F_INNER_MAP flag
set, won't this allow the verifier to still incorrectly elide NULLness
checks for per-CPU arrays used as inner map templates?

If a per-CPU array with a large max_entries is used as an inner map
template, and a concrete map with a smaller max_entries is inserted at
runtime (which is possible because percpu_array_map_ops.map_meta_equal uses
bpf_map_meta_equal() which ignores max_entries checks), couldn't this
lead to an out-of-bounds access or NULL pointer dereference?

Additionally, since percpu_array_map_gen_lookup() also relies on
BPF_F_INNER_MAP to skip JIT inlining, will it inline the lookup with the
template's excessively large bounds hardcoded, resulting in out-of-bounds
heap reads on the concrete inner map?

>  		    meta.const_map_key >= 0 &&
>  		    meta.const_map_key < meta.map.ptr->max_entries)
>  			ret_flag &= ~PTR_MAYBE_NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624072031.735846-1-sun.jian.kdev@gmail.com?part=1