Re: [PATCH bpf-next 1/2] selftests/bpf: Return the lock error from buddy_init() of libarena

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: missing ASAN poisoning on chunk free in buddy_init() error path
- [Low] bpf: libarena leak of reserved vaddr on buddy_init lock failure
--

commit 38d07f939b85ac6851d3855b00b730195ca11736
Author: Qiliang Yuan <odys.yuan@gmail.com>

selftests/bpf: Return the lock error from buddy_init() of libarena

This commit modifies buddy_init() to return the specific lock error from
buddy_lock() rather than unconditionally returning -EINVAL. This helps
distinguish between -EOPNOTSUPP, -ETIMEDOUT, and -EBUSY cases.

> diff --git a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c
> index 2490ab1396de0..7bb99a7cb63ed 100644
> --- a/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c
> +++ b/tools/testing/selftests/bpf/libarena/src/buddy.bpf.c
> @@ -576,9 +576,10 @@ __weak int buddy_init(struct buddy __arena *buddy)
>  
>  	chunk = buddy_chunk_get(buddy);
>  
> -	if (buddy_lock(buddy, flags)) {
> +	ret = buddy_lock(buddy, flags);
> +	if (ret) {
>  		bpf_arena_free_pages(&arena, chunk, BUDDY_CHUNK_PAGES);

[Severity: Low]
Does this code leak the reserved arena virtual address space?

Earlier in buddy_init(), buddy_reserve_arena_vaddr() is called to reserve
enough address space. If buddy_lock() subsequently fails, the error path
frees the allocated chunk, but it does not call buddy_unreserve_arena_vaddr()
to clean up the reserved space before returning the error.

[Severity: Medium]
Could this error path leave the freed chunk memory improperly poisoned?

When buddy_chunk_get() successfully allocates a chunk, it partially unpoisons
and poisons the memory for ASAN. In buddy_destroy(), the chunk is correctly
repoisoned by calling asan_poison(chunk, BUDDY_POISONED, ...) before the
memory is freed.

If buddy_lock() fails here and bpf_arena_free_pages() frees the chunk without
first re-poisoning it, does this leave the ASAN shadow memory in a stale
state that might cause false positives or negatives if the pages are
subsequently reallocated within the same arena?

> -		return -EINVAL;
> +		return ret;
>  	}
>  
>  	/* Chunk is already properly unpoisoned if allocated. */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-libarena-nr-cpus-v1-0-e726ccc66fc6@gmail.com?part=1