Re: [PATCH bpf-next 2/2] selftests/bpf: Cover RHash special-field recycle

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The RHash map implementation leaks BTF special fields (such as kptrs) during in-place updates and element recycling by failing to call bpf_obj_free_fields().
- [Medium] The new selftest test_rhash_special_fields_recycle incorrectly asserts and enforces a kernel memory leak regarding BTF special fields.
--

--- Patch [2]: [PATCH bpf-next 2/2] selftests/bpf: Cover RHash special-field recycle ---
commit d7b4034e22767ea5cd89689a56855953c3b5c45a
Author: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>

selftests/bpf: Cover RHash special-field recycle

This commit adds regression coverage for RHash value recycling with
referenced kptrs. It stores a referenced task kptr and updates the value
with BPF_EXIST, then verifies that bpf_kptr_xchg() still returns the kptr.
Finally, it deletes the element to exercise the delayed reclamation path.

> diff --git a/tools/testing/selftests/bpf/progs/rhash.c b/tools/testing/selftests/bpf/progs/rhash.c
> index fc2dac3a719e9..d28d115cb74e7 100644
> --- a/tools/testing/selftests/bpf/progs/rhash.c
> +++ b/tools/testing/selftests/bpf/progs/rhash.c

[ ... ]

> +	err = 5;
> +	if (bpf_map_update_elem(&special_fields, &key, &val2, BPF_EXIST))
> +		return 5;
> +
> +	err = 6;
> +	elem = bpf_map_lookup_elem(&special_fields, &key);
> +	if (!elem)
> +		return 6;
> +
> +	err = 7;
> +	task = bpf_kptr_xchg(&elem->task, NULL);
> +	if (!task)
> +		return 7;

[Severity: Medium]
Is it intentional for this test to codify a kernel memory leak as a valid
feature?

The test updates the element using bpf_map_update_elem() with val2 (which
has no task kptr) but then expects the old kptr to still be present and valid
in the map.

According to the BPF subsystem guidelines, when a map element is overwritten,
bpf_obj_free_fields() must be called to release resources held by the old
value. By asserting that the old kptr survives the map update, the test
appears to enforce a bug where bpf_obj_free_fields() is missing.

[Severity: High]
Could this highlight a resource leak in the underlying RHash map
implementation?

Looking at the RHash implementation in kernel/bpf/hashtab.c,
rhtab_map_update_existing() appears to overwrite the old value using
copy_map_value() but misses the required bpf_obj_free_fields() call.

Additionally, when deleting an element:

kernel/bpf/hashtab.c:rhtab_delete_elem() {
    ...
    /* Cancel reusable internal structs: bpf_timer, task_work, wq */
    rhtab_check_and_cancel_fields(rhtab, elem);
    ...
}

It seems to skip freeing kptrs, leaking the references. Should
bpf_obj_free_fields() be called during these operations to properly release
kernel resources like task_structs?

> +	bpf_task_release(task);
> +
> +	err = 8;
> +	task = bpf_task_acquire(bpf_get_current_task_btf());
> +	if (!task)
> +		return 8;

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