Re: [PATCH bpf-next v4 05/11] bpf: Allow special fields in resizable hashtab

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 5/14/26 12:21 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
>> index 61eb88cb9229..9cc41850dc79 100644
>> --- a/kernel/bpf/hashtab.c
>> +++ b/kernel/bpf/hashtab.c
> 
> [ ... ]
> 
>> @@ -2943,6 +2965,7 @@ static int rhtab_map_lookup_and_delete_elem(struct bpf_map *map, void *key, void
>>  static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *elem, void *value,
>>  				      u64 map_flags)
>>  {
>> +	struct bpf_rhtab *rhtab = container_of(map, struct bpf_rhtab, map);
>>  	void *old_val = rhtab_elem_value(elem, map->key_size);
>>
>>  	if (map_flags & BPF_NOEXIST)
>> @@ -2952,6 +2975,13 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>>  		copy_map_value_locked(map, old_val, value, false);
>>  	else
>>  		copy_map_value(map, old_val, value);
>> +
>> +	/*
>> +	 * 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.
>> +	 */
>> +	rhtab_check_and_free_fields(rhtab, elem);
>>  	return 0;
>>  }
> 
> Could calling rhtab_check_and_free_fields() after copy_map_value() create
> a race with concurrent BPF programs?

Same as arraymap, yes, there is a race.
> 
> When copy_map_value() skips special field offsets, the old timer/kptr/etc.
> remains active in the slot during the copy. If a concurrent program acquires
> the spinlock (in the BPF_F_LOCK case via copy_map_value_locked) or just
> accesses the value between copy and free operations, couldn't it still use
> those special fields even after rhtab_check_and_free_fields() cancels them?
> 
> In v3 of this series, sashiko-bot raised this concern about the ordering
> of these operations. While the patch moved rhtab_check_and_free_fields()
> to occur after the copy (matching arraymap semantics), the fundamental
> timing window between copy and free still allows concurrent access to
> special fields that are about to be canceled.
> 
> [ ... ]
> 
> Commit message:
> 
>>     bpf: Allow special fields in resizable hashtab
>>
>>     Add support for timers, workqueues, task work and spin locks.
>>     Without this, users needing deferred callbacks or BPF_F_LOCK in a
>>     dynamically-sized map have no option - fixed-size htab is the only
>>     map supporting these field types. Resizable hashtab should offer the
>>     same capability.
>>
>>     Properly clean up BTF record fields on element delete and map
>>     teardown by wiring up bpf_obj_free_fields through a memory allocator
>>     destructor, matching the pattern used by htab for non-prealloc maps.
>>
>>     Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> 
> This looks like a bug fix for resizable hashtab support. Should this
> include:
> 
>   Fixes: 9cf2facd6b40 ("bpf: Implement resizable hashmap basic functions")
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25830929099