Re: [PATCH bpf-next v3 4/4] selftests/bpf: Exercise kptr map update lifetime

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The newly added selftests (`test_array_map_update_kptr` and `test_hash_map_update_kptr`) codify a preexisting bug where `bpf_map_update_elem` fails to free kptrs in hash and array maps. The tests assert that the kptr refcount remains elevated (`check_refs(3)`) after the update, incorrectly locking in the broken BTF field lifecycle behavior.
--

commit ca10a096e58e2f06a9a6605b1b8ff02a49d2d192
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Exercise kptr map update lifetime

This commit adds focused map_kptr coverage for BPF-side map updates that
touch values containing referenced kptrs. The test stashes refcounted objects
in array and hash maps, then updates them from BPF to verify the refcount
behavior during map teardown.

> diff --git a/tools/testing/selftests/bpf/progs/map_kptr.c b/tools/testing/selftests/bpf/progs/map_kptr.c
> index e708ffbe1f610..3fbefc568e0a6 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr.c
[ ... ]
> @@ -500,11 +499,95 @@ int count_ref(void *ctx)
[ ... ]
> +SEC("syscall")
> +int test_array_map_update_kptr(void *ctx)
> +{
> +	struct map_value init = {}, *v;
> +	int key = 0, ret;
> +
> +	v = bpf_map_lookup_elem(&array_map, &key);
> +	if (!v)
> +		return 1;
> +	ret = stash_ref_ptr(v);
> +	if (ret)
> +		return ret;
> +	ret = check_refs(3);
> +	if (ret)
> +		return ret;
> +	ret = bpf_map_update_elem(&array_map, &key, &init, BPF_EXIST);
> +	if (ret)
> +		return 4;
> +	return check_refs(3);

[Severity: High]
This is a pre-existing issue, but does this test codify a bug where
bpf_map_update_elem() fails to free kptrs in hash and array maps?

When a map element is overwritten with an empty value, shouldn't the kptr
reference count drop? According to the BTF field lifecycle guidelines, map
updates that overwrite existing values with copy_map_value() must call
bpf_obj_free_fields() to release resources held by the old value.

However, it appears array_map_update_elem() in kernel/bpf/arraymap.c calls
bpf_obj_cancel_fields():

kernel/bpf/arraymap.c:array_map_update_elem() {
    ...
    copy_map_value(map, val, value);
    bpf_obj_cancel_fields(map, val);
    ...
}

And free_htab_elem() in kernel/bpf/hashtab.c calls
check_and_cancel_fields() before pushing to the freelist:

kernel/bpf/hashtab.c:free_htab_elem() {
    ...
    check_and_cancel_fields(htab, l);
    pcpu_freelist_push(&htab->freelist, &l->fnode);
    ...
}

Both of these paths skip kptr cleanup.

By asserting that the refcount remains unchanged at 3 after the update in
test_array_map_update_kptr() and test_hash_map_update_kptr(), the tests appear
to incorrectly lock in the broken BTF field lifecycle behavior where array
kptrs leak and preallocated hash map kptrs are resurrected when reallocated.

> +}

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