Re: [PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle

[sashiko-bot@kernel.org]

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

New issues:
- [High] The `rhtab_map_update_existing` function fails to call `bpf_obj_free_fields()` when overwriting an existing map value, explicitly violating BTF special field cleanup rules.

Pre-existing issues:
- [Critical] Uninitialized spinlock acquired in `alloc_htab_elem` when `BPF_F_LOCK` is used.
- [Critical] BTF special field (kptr) leak and dangerous reuse in preallocated hash maps.
- [High] Missing field cancellation on locked update in `htab_map_update_elem`.
--

--- Patch [1]: [PATCH bpf-next 1/2] bpf: Cancel RHash special fields on value recycle ---
commit b285c707d0a30a431d69802916cd425986e713ab
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

bpf: Cancel RHash special fields on value recycle

The patch replaces bpf_obj_free_fields() with bpf_obj_cancel_fields() in
the update and deferred deletion paths for rhtab. It cancels timers and
workqueues while deferring kptr release.

> diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
> index 9f394e1aa2e85..54ea111daa8bd 100644
> --- a/kernel/bpf/hashtab.c
> +++ b/kernel/bpf/hashtab.c
[ ... ]
> @@ -3027,7 +3027,7 @@ static long rhtab_map_update_existing(struct bpf_map *map, struct rhtab_elem *el
>  	 * 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);
> +	rhtab_check_and_cancel_fields(rhtab, elem);
>  	return 0;
>  }

[Severity: High]
Does replacing bpf_obj_free_fields() with bpf_obj_cancel_fields() violate
the subsystem guidelines for map updates?

According to the BPF documentation, when overwriting an existing map value
with userspace data using copy_map_value(), bpf_obj_free_fields() must be
called to release resources held by the old value. 

Does using bpf_obj_cancel_fields() here instead fail to release resources
like kptrs during the update?


[Severity: High]
This is a pre-existing issue, but are special fields left active during
locked updates in htab_map_update_elem()?

Looking at kernel/bpf/hashtab.c:htab_map_update_elem():

    if (map_flags & BPF_F_LOCK) {
        ...
        copy_map_value_locked(map, htab_elem_value(l_old, key_size), value, false);
        return 0;
    }

When BPF_F_LOCK is used to overwrite an existing element in-place,
copy_map_value_locked() is called and the function returns 0. 

Is the cancellation step skipped here, leaving old special fields like
timers or workqueues running indefinitely?


[Severity: Critical]
This is a pre-existing issue, but could alloc_htab_elem() be acquiring an
uninitialized spinlock?

Looking at kernel/bpf/hashtab.c:alloc_htab_elem():

    l_new = bpf_mem_cache_alloc(&htab->ma);
    ...
    if (map_flags & BPF_F_LOCK)
        copy_map_value_locked(&htab->map, htab_elem_value(l_new, key_size), value, false);

When updating a non-preallocated hash map with BPF_F_LOCK for a new key,
the new element is allocated but not initialized. 

Because check_and_init_map_value() is not called before
copy_map_value_locked(), will the kernel attempt to lock an uninitialized
spinlock in the new memory?


[Severity: Critical]
This is a pre-existing issue, but can preallocated hash maps leak kptrs
and improperly reuse them across elements?

When an element is deleted, kernel/bpf/hashtab.c:free_htab_elem() cancels
timers but doesn't free fields:

    check_and_cancel_fields(htab, l);
    pcpu_freelist_push(&htab->freelist, &l->fnode);

Later, kernel/bpf/hashtab.c:alloc_htab_elem() pops the element and skips
initialization:

    l_new = prealloc_lru_pop(htab, key, hash);
    if (!l_new)
        ...

Because bpf_obj_free_fields() is skipped on deletion and
check_and_init_map_value() is skipped on reallocation, would the new
element inherit the old kptrs? 

Might this cause kptr resources to leak or be improperly accessed by the
reallocated element?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726-f01-23-rhash-cancel-bpf-next-v1-0-6e5e1131d885@mails.tsinghua.edu.cn?part=1