Re: [PATCH bpf v3 6/7] bpf: Assign lock identity to callback map values

[Eduard Zingerman <eddyz87@gmail.com>]

On Mon, 2026-09-14 at 15:16 +0200, Kumar Kartikeya Dwivedi wrote:
> A nested bpf_for_each_map_elem() callback can unlock a different element
> of the same map:
> 
>   static long inner(void *map, int *key, struct value *v,
>                     struct value **outer_value)
>   {
>           bpf_spin_lock(&v->lock);
>           bpf_spin_unlock(&(*outer_value)->lock);
>           return 0;
>   }
> 
>   static long outer(void *map, int *key, struct value *v, void *ctx)
>   {
>           bpf_for_each_map_elem(map, inner, &v, 0);
>           return 0;
>   }
> 
> Both callback values currently have ID zero and the same map_ptr.
> process_spin_lock() compares those two fields, so it accepts the unlock
> even though the two callbacks can receive different map elements.
> 
> Assign a fresh ID to every callback map value in the for-each,
> timer/workqueue, and task-work constructors. Copies of one callback
> argument retain its ID, so locking and unlocking through that argument
> continues to work. Distinct callbacks also get distinct IDs for
> single-element arrays, including inner arrays sharing inner_map_meta.
> 
> Preserve map_uid for every inner-map lookup and compare it through
> check_ids() during state pruning. This preserves relationships between
> maps, keys, and values while allowing equivalent states with different
> lookup IDs to match. It avoids field-specific rules for when an inner map
> needs an identity.
> 
> Move map_uid out of the metadata union and next to the other IDs, so
> register comparisons can use the existing memcmp() ranges and remap the
> IDs separately. Clear it when resetting a register or converting a map
> lookup result to a socket pointer. Shrink frameno to u8, which is enough
> for MAX_CALL_FRAMES, to make room without growing bpf_reg_state.
> 
> Fixes: d0d78c1df9b1 ("bpf: Allow locking bpf_spin_lock global variables")
> Reported-by: Nicholas Carlini <npc@anthropic.com>
> Suggested-by: Nicholas Carlini <npc@anthropic.com>
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

It appears all map_uid has to be reset are handled.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index ddba53eaa333..ab4f0ea2647b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

...

> @@ -1925,17 +1926,18 @@ static void refine_map_lookup_value(struct bpf_reg_state *reg)
>  	if (map->inner_map_meta) {
>  		reg->type = CONST_PTR_TO_MAP | maybe_null;
>  		reg->map_ptr = map->inner_map_meta;
> -		/* transfer reg's id which is unique for every map_lookup_elem
> -		 * as UID of the inner map.
> +		/*
> +		 * Each lookup gets a unique ID. Preserve it as the inner map's
> +		 * identity, since different inner maps share inner_map_meta.
>  		 */

Nit: old comment is better, imo.

> -		if (btf_record_has_field(map->inner_map_meta->record,
> -					 BPF_TIMER | BPF_WORKQUEUE | BPF_TASK_WORK))
> -			reg->map_uid = reg->id;
> +		reg->map_uid = reg->id;
>  	} else if (map->map_type == BPF_MAP_TYPE_XSKMAP) {
>  		reg->type = PTR_TO_XDP_SOCK | maybe_null;
> +		reg->map_uid = 0;
>  	} else if (map->map_type == BPF_MAP_TYPE_SOCKMAP ||
>  		   map->map_type == BPF_MAP_TYPE_SOCKHASH) {
>  		reg->type = PTR_TO_SOCKET | maybe_null;
> +		reg->map_uid = 0;
>  	}
>  }
>  

...