Re: [RFC PATCH bpf-next v1 16/16] selftests/bpf: Exercise typed arenas at run time

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/bpf: test_release_bounds unconditionally fails due to logic error in alloc_at expectation
--

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

selftests/bpf: Exercise typed arenas at run time

Drive typed arenas from user space through a skeleton, testing map memory
usage, chunk allocation, release, multipage allocations, bounds checking,
and pointer fields.

> diff --git a/tools/testing/selftests/bpf/prog_tests/typed_arena.c b/tools/testing/selftests/bpf/prog_tests/typed_arena.c
> new file mode 100644
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/typed_arena.c

[ ... ]

> +static void test_release_bounds(void)
> +{
> +	long ps = PAGE_SIZE, base, nr = OBJ_ARENA_SIZE / PAGE_SIZE;
> +	struct typed_arena *skel;
> +	int fd;
> +
> +	skel = typed_arena__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_load"))
> +		return;
> +	fd = bpf_map__fd(skel->maps.arena);
> +	base = map_memlock(fd);
> +
> +	/* Fill the typed arena, so that a scan of its bitmap for a free chunk runs to the end. */
> +	skel->bss->ptr = NULL;
> +	skel->data->page_cnt = nr;
> +	if (run(skel->progs.alloc_at, "fill"))
> +		goto out;

[Severity: Medium]
Will this unconditionally abort the test?

When skel->bss->ptr is set to NULL in test_release_bounds(), the BPF program
alloc_at() executes with its hint variable as NULL. If the allocation is
successful, o will be non-NULL, which makes the condition o != hint evaluate
to 1 (true):

tools/testing/selftests/bpf/progs/typed_arena.c:alloc_at() {
    ...
	hint = ptr;
	o = bpf_typed_arena_alloc_pages(&arena, TYPE_ID(struct obj), hint, &page_cnt, NUMA_NO_NODE);
	if (!o)
		return 1;
	return o != hint;
}

The run() helper wraps ASSERT_OK(ret), which expects a return value of 0.
Since it receives 1, the assertion fails and the test executes goto out,
leaving the rest of test_release_bounds() unexecuted.

Could the return logic in alloc_at() be adjusted when hint is NULL, or
should this test use a different program or helper to fill the arena?

> +	ASSERT_EQ(map_memlock(fd), base + nr * ps, "full");
> +
> +	skel->data->page_cnt = nr + 1;
> +	if (run(skel->progs.free_pages, "free_oversized"))
> +		goto out;

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