Re: [PATCH RESEND bpf-next v10 8/8] selftests/bpf: Add test cases for bpf_list_del/add/is_first/is_last/empty

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Latent verifier errors (use-after-release and memory leak) in the negative test `list_add_without_lock_fail`.
--

commit 37523e4b94785be6995dc6c7cf660c200eca4aef
Author: Kaitao Cheng <chengkaitao@kylinos.cn>

selftests/bpf: Add test cases for bpf_list_del/add/is_first/is_last/empty

This commit extends the refcounted_kptr selftests by adding new test cases
for BPF linked list operations. It includes both successful path tests and
negative tests verifying that the verifier correctly rejects operations
made without holding the required spinlock.

> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> index c847398837ccd..21ae06797b183 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c

[ ... ]

> +SEC("?tc")
> +__failure __msg("bpf_spin_lock at off=32 must be held for bpf_list_head")
> +long list_add_without_lock_fail(void *ctx)
> +{
> +	struct node_data *n, *prev;
> +
> +	n = bpf_obj_new(typeof(*n));
> +	if (!n)
> +		return -1;
> +
> +	prev = bpf_obj_new(typeof(*prev));
> +	if (!prev) {
> +		bpf_obj_drop(n);
> +		return -1;
> +	}
> +
> +	/* Error case: add list node without holding lock */
> +	if (bpf_list_add(&head, &n->l, &prev->l)) {
> +		bpf_obj_drop(prev);
> +		bpf_obj_drop(n);

Since bpf_list_add() consumes the newly added node (&n->l), would this
subsequent bpf_obj_drop(n) trigger a use-after-release error in the
verifier if it evaluates past the missing spinlock?

> +		return -2;
> +	}
> +
> +	return 0;

Similarly, prev is an owning reference acquired via bpf_obj_new(), but
bpf_list_add() accepts a non-owning reference for the prev argument. Does
this code leak the prev reference by returning 0 here without calling
bpf_obj_drop(prev)?

While the test correctly fails today due to the missing spinlock as
intended, these latent errors could make the test brittle and cause CI
flakes if the verifier's evaluation order changes in the future.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512055919.95716-1-kaitao.cheng@linux.dev?part=8