[PATCH bpf-next 0/2] selftests/bpf: Skip libarena when arena spin locks are unsupported

[PATCH bpf-next 0/2] selftests/bpf: Skip libarena when arena spin locks are unsupported

[Qiliang Yuan]

serial_test_libarena runs the test programs of libarena, whose buddy
allocator serializes its operations with an arena spin lock.

arena_spin_lock() fails with -EOPNOTSUPP on kernels built with more
than 1024 possible CPUs, as distributions commonly configure them.
buddy_init() reports that as -EINVAL, and every test then fails at
arena_buddy_reset.

Patch 1 makes buddy_init() return the error of the lock, patch 2 skips
the tests when resetting the allocator fails with -EOPNOTSUPP.

Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
Qiliang Yuan (2):
      selftests/bpf: Return the lock error from buddy_init() of libarena
      selftests/bpf: Skip libarena on kernels with too many possible CPUs

 tools/testing/selftests/bpf/libarena/src/buddy.bpf.c | 5 +++--
 tools/testing/selftests/bpf/prog_tests/libarena.c    | 9 +++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)
---
base-commit: e1d84a37cba984388988d2f1ddc84561413f0db2
change-id: 20261009-selftests-bpf-libarena-nr-cpus-50fc5dd00fe2

Best regards,
-- 
Qiliang Yuan <odys.yuan@gmail.com>

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

[Qiliang Yuan]

buddy_init() takes the buddy lock, an arena spin lock, to put its first
chunk on the list, and returns -EINVAL when it can't.

arena_spin_lock() fails with -EOPNOTSUPP on kernels built with more
than 1024 possible CPUs, where it has no queue node for every CPU, and
with -ETIMEDOUT or -EBUSY otherwise. Returning -EINVAL for all of them
hides which one happened from the caller.

Return the error of the lock.

Fixes: b1487dc1b181 ("selftests/bpf: Add selftests for libarena buddy allocator")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 tools/testing/selftests/bpf/libarena/src/buddy.bpf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

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);
-		return -EINVAL;
+		return ret;
 	}
 
 	/* Chunk is already properly unpoisoned if allocated. */

-- 
2.43.0

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

[sashiko-bot]

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

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2176 bytes --]

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

TL;DR: Commit message only: the Fixes: tag may point at the wrong commit
(b1487dc1b181, which doesn't touch buddy_init()); 86426a28c52d introduced the
code being changed.

>     selftests/bpf: Return the lock error from buddy_init() of libarena
>
>     buddy_init() takes the buddy lock, an arena spin lock, to put its first
>     chunk on the list, and returns -EINVAL when it can't.
>     arena_spin_lock() fails with -EOPNOTSUPP on kernels built with more
>     than 1024 possible CPUs, where it has no queue node for every CPU, and
>     with -ETIMEDOUT or -EBUSY otherwise. Returning -EINVAL for all of them
>     hides which one happened from the caller.
>     Return the error of the lock.
>     Fixes: b1487dc1b181 ("selftests/bpf: Add selftests for libarena buddy allocator")
>     Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>

Is the Fixes: tag pointing at the right commit?

b1487dc1b181 ("selftests/bpf: Add selftests for libarena buddy allocator")
only adds st_buddy.bpf.c, st_asan_buddy.bpf.c, prog_tests/libarena.c and
prog_tests/libarena_asan.c.  It does not touch src/buddy.bpf.c or
buddy_init().

The code being changed here:

    if (buddy_lock(buddy, flags)) {
        bpf_arena_free_pages(&arena, chunk, BUDDY_CHUNK_PAGES);
        return -EINVAL;
    }

was introduced by 86426a28c52d ("selftests/bpf: Add buddy allocator for
libarena"), which added buddy.bpf.c with buddy_init() already returning
-EINVAL when buddy_lock() failed.  The selftests commit only exposes the
discarded error, it did not introduce the code that discards it.

The same tag is also used by the follow-up commit 27aeee1ef85a
("selftests/bpf: Skip libarena on kernels with too many possible CPUs"),
which depends on this patch to see -EOPNOTSUPP from arena_buddy_reset().

Should this be:

  Fixes: 86426a28c52d ("selftests/bpf: Add buddy allocator for libarena")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37873620927

[PATCH bpf-next 2/2] selftests/bpf: Skip libarena on kernels with too many possible CPUs

[Qiliang Yuan]

serial_test_libarena runs every test program of libarena, most of them
after resetting the buddy allocator with arena_buddy_reset.

The buddy allocator takes an arena spin lock, which fails with
-EOPNOTSUPP on kernels built with more than 1024 possible CPUs, as
distributions commonly configure them. Every test then fails at
arena_buddy_reset, while the arena_spin_lock test already skips in that
case.

Reset the allocator once before running the tests, and skip them when
that returns -EOPNOTSUPP.

Fixes: b1487dc1b181 ("selftests/bpf: Add selftests for libarena buddy allocator")
Signed-off-by: Qiliang Yuan <odys.yuan@gmail.com>
---
 tools/testing/selftests/bpf/prog_tests/libarena.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/libarena.c b/tools/testing/selftests/bpf/prog_tests/libarena.c
index daade4150af66..070a08856ee04 100644
--- a/tools/testing/selftests/bpf/prog_tests/libarena.c
+++ b/tools/testing/selftests/bpf/prog_tests/libarena.c
@@ -240,6 +240,15 @@ void serial_test_libarena(void)
 	if (!ASSERT_OK(ret, "arena_alloc_reserve"))
 		goto out;
 
+	/* Skip on kernels with more possible CPUs than arena spin locks support */
+	ret = libarena_run_prog(bpf_program__fd(skel->progs.arena_buddy_reset));
+	if (ret == -EOPNOTSUPP) {
+		test__skip();
+		goto out;
+	}
+	if (!ASSERT_OK(ret, "arena_buddy_reset"))
+		goto out;
+
 	bpf_object__for_each_program(prog, skel->obj) {
 		const char *name = bpf_program__name(prog);
 

-- 
2.43.0

Re: [PATCH bpf-next 2/2] selftests/bpf: Skip libarena on kernels with too many possible CPUs

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2022 bytes --]

> selftests/bpf: Skip libarena on kernels with too many possible CPUs

TL;DR: Nit only: the libarena selftest skips silently on too many possible
CPUs; suggests printing a reason first, as arena_spin_lock.c does, though
leaving it may be acceptable.

> diff --git a/tools/testing/selftests/bpf/prog_tests/libarena.c b/tools/testing/selftests/bpf/prog_tests/libarena.c
> index daade4150af6..070a08856ee0 100644
> --- a/tools/testing/selftests/bpf/prog_tests/libarena.c
> +++ b/tools/testing/selftests/bpf/prog_tests/libarena.c

[ ... ]

> @@ -240,6 +240,15 @@ void serial_test_libarena(void)
>  	if (!ASSERT_OK(ret, "arena_alloc_reserve"))
>  		goto out;
>
> +	/* Skip on kernels with more possible CPUs than arena spin locks support */
> +	ret = libarena_run_prog(bpf_program__fd(skel->progs.arena_buddy_reset));
> +	if (ret == -EOPNOTSUPP) {
> +		test__skip();
> +		goto out;
> +	}
> +	if (!ASSERT_OK(ret, "arena_buddy_reset"))
> +		goto out;
> +

This isn't a bug, but a subsystem guideline flags a test that skips for a
missing capability without printing any reason.

When arena_buddy_reset returns -EOPNOTSUPP, this path calls a bare
test__skip(), so the test log shows SKIP with nothing indicating why.  The
explanation only exists in the source comment above the call.

The sibling prog_tests/arena_spin_lock.c prints a reason before skipping
for the same condition:

    printf("%s:SKIP: CONFIG_NR_CPUS exceed the maximum supported by arena spinlock\n",
           __func__);
    test__skip();

Would it make sense to print a similar message here before test__skip()?

On the other hand, test__skip() takes no message argument, and the existing
skip in run_libarena_parallel_test() for a disabled parallel test is also
bare, so leaving it as is may be acceptable.

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/37873620927