Re: [PATCH bpf v1 7/8] bpf: Assign lock identity to callback map values

[sashiko-bot@kernel.org]

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

New issues:
- [High] Lock identity aliasing across distinct inner map instances due to flawed static identity check.

Pre-existing issues:
- [High] A BPF program can cause an AA deadlock by acquiring `bpf_res_spin_lock` twice on the same single-element array map element, bypassing verifier lock tracking.
--

commit 996188899c474047a5a9bb68f80d0d3c3e5a5b6a
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Assign lock identity to callback map values

This commit assigns a fresh verifier ID to lockable map values in
callback frame constructors to prevent lock aliasing. It specifically
preserves ID zero for single-element array maps to support stable
allocation aliases.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 1c3039f3fc323..b08501734ddfa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -447,6 +447,11 @@ static bool reg_may_point_to_spin_lock(const struct bpf_reg_state *reg)
>  	return btf_record_has_field(reg_btf_record(reg), BPF_SPIN_LOCK | BPF_RES_SPIN_LOCK);
>  }
>  
> +static bool map_value_has_static_identity(const struct bpf_map *map)
> +{
> +	return map->map_type == BPF_MAP_TYPE_ARRAY && map->max_entries == 1;
> +}

[Severity: High]
Does this check fail to distinguish inner_map_meta from an actual
single-element array map?

When a BPF program retrieves two distinct inner maps from an outer map
(where the inner maps are single-element array maps), and calls
bpf_for_each_map_elem on both, the verifier tracks both using the same
inner_map_meta.

Because inner_map_meta has max_entries == 1 and map_type ==
BPF_MAP_TYPE_ARRAY, will map_value_has_static_identity() return true and
incorrectly assign id = 0 to both callbacks' element pointers?

Could this allow a program to acquire a lock on an element from the first
inner map and unlock it on the element from the second inner map, corrupting
the spinlock state?

[Severity: High]
This is a pre-existing issue, but does preserving ID 0 for single-element
array maps leave an AA deadlock vulnerability in bpf_res_spin_lock tracking?

If a program acquires a bpf_res_spin_lock using a direct map value pointer
via LD_IMM64, check_ld_imm() assigns id = 0. If the program then acquires
the same lock again using a pointer obtained via bpf_map_lookup_elem(),
check_helper_call() unconditionally assigns a fresh ID to the lookup result.

Will find_lock_state() inside process_spin_lock() see the different IDs and
fail to recognize the lock is already held, causing a system hang on the
second acquisition?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905070003.3193366-1-memxor@gmail.com?part=7