Re: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing `bpf_obj_free_fields()` in map updates, directly violating BPF-001 guidelines.
--

--- Patch [1]: [PATCH 1/4] bpf: Cancel special fields in resizable hashtab on recycle ---
commit 6f12e4dc235993c2415d956e168783cb133ed13a
Author: Yuan Chen <[email protected]>

bpf: Cancel special fields in resizable hashtab on recycle

This commit updates the resizable hashtab recycle paths to cancel only
NMI-safe fields instead of fully freeing all fields. This is intended to
prevent kptr destructors from running in NMI contexts during map updates or
deletions, delaying their destruction until the element is actually freed.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -3022,10 +3064,11 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * BPF_F_LOCK, matching arraymap semantics.
>  	 *
>  	 * copy_map_value() skips special-field offsets, so old timers/
> -	 * kptrs/etc. still sit in the slot. Cancel them after the copy
> -	 * to match arraymap's update semantics.
> +	 * kptrs/etc. still sit in the slot. Cancel the NMI-safe ones after
> +	 * the copy to match arraymap's update semantics; referenced kptrs
> +	 * stay attached and are destroyed by rhtab_mem_dtor().
>  	 */
> -	rhtab_check_and_free_fields(rhtab, elem);
> +	rhtab_cancel_fields(rhtab, elem);

[Severity: Medium]
Does this bypass the required resource cleanup for overwritten map values?

The BPF subsystem guidelines mandate that when overwriting an existing map
value using copy_map_value(), bpf_obj_free_fields() must be called to release
resources held by the old value before or after the copy overwrites them.

Since rhtab_cancel_fields() is used here instead of bpf_obj_free_fields(),
could this leak kptrs or other resources when the old value is overwritten
during a map update?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1