Re: [PATCH bpf-next v1 3/5] bpf: Cancel special fields on map value recycle

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Replacing bpf_obj_free_fields() with bpf_obj_cancel_fields() in map update and delete paths fails to clear BTF special fields (kptrs, list heads, etc.), causing recycled map elements to silently inherit these fields from previously deleted or overwritten keys.
--

commit 0b4e0636a1d77531077ad0491c76e6b6595a28c6
Author: Justin Suess <utilityemal77@gmail.com>

bpf: Cancel special fields on map value recycle

This patch introduces bpf_obj_cancel_fields() to replace
bpf_obj_free_fields() in map update and delete paths. It restricts
eager cleanup to timers, workqueues, and task works, leaving other
fields attached for final map destruction.

> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index e6271a2bf6d64..4e8d6fe6999bf 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -384,7 +384,7 @@ static long array_map_update_elem(struct bpf_map *map, void *key, void *value,
>  	if (array->map.map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
>  		val = this_cpu_ptr(array->pptrs[index & array->index_mask]);
>  		copy_map_value(map, val, value);
> -		bpf_obj_free_fields(array->map.record, val);
> +		bpf_obj_cancel_fields(array->map.record, val);
>  	} else {
>  		val = array->value +
>  			(u64)array->elem_size * (index & array->index_mask);
> @@ -392,7 +392,7 @@ static long array_map_update_elem(struct bpf_map *map, void *key, void *value,
>  			copy_map_value_locked(map, val, value, false);
>  		else
>  			copy_map_value(map, val, value);
> -		bpf_obj_free_fields(array->map.record, val);
> +		bpf_obj_cancel_fields(array->map.record, val);

[Severity: High]
Does this prevent userspace from clearing kptrs in array maps?

Because bpf_obj_cancel_fields() leaves kptrs untouched and copy_map_value()
skips copying BTF fields from the user, array elements are updated without
freeing or zeroing old kptrs.

This permanently attaches the kptr to the array slot until map destruction,
making it impossible to clear them by updating the element with zeroes.

[ ... ]

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index b4366cad3cfa5..a9b2b882b90f2 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -833,8 +837,8 @@ static int htab_lru_map_gen_lookup(struct bpf_map *map,
>  	return insn - insn_buf;
>  }
>  
> -static void check_and_free_fields(struct bpf_htab *htab,
> -				  struct htab_elem *elem)
> +static void check_and_cancel_fields(struct bpf_htab *htab,
> +				    struct htab_elem *elem)
>  {
>  	if (IS_ERR_OR_NULL(htab->map.record))
>  		return;
> @@ -844,11 +848,11 @@ static void check_and_free_fields(struct bpf_htab *htab,
>  		int cpu;
>  
>  		for_each_possible_cpu(cpu)
> -			bpf_obj_free_fields(htab->map.record, per_cpu_ptr(pptr, cpu));
> +			bpf_obj_cancel_fields(htab->map.record, per_cpu_ptr(pptr, cpu));
>  	} else {
>  		void *map_value = htab_elem_value(elem, htab->map.key_size);
>  
> -		bpf_obj_free_fields(htab->map.record, map_value);
> +		bpf_obj_cancel_fields(htab->map.record, map_value);
>  	}
>  }

[Severity: High]
Does this allow recycled hash map elements to silently inherit kptrs from
previously deleted keys?

When an element is deleted, it goes back to the freelist or memory allocator
cache. Since check_and_cancel_fields() no longer clears BTF special fields
like kptrs or list heads, these fields remain intact in the recycled memory.

When alloc_htab_elem() reuses this memory for a new key, copy_map_value()
intentionally skips over BTF fields.

The new element will thus silently inherit the kptr from the old element,
breaking cross-key isolation.

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