Re: [PATCH bpf-next v2 4/4] selftests/bpf: Add tests for memory usage for arena

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 7/17/26 7:53 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Incorrect use of ASSERT_OK_PTR to check for a NULL pointer read from BPF userspace memory, breaking intended error handling.
> --
>
> --- Patch [4]: [PATCH bpf-next v2 4/4] selftests/bpf: Add tests for memory usage for arena ---
> commit b8bd79585d9f7cbf2a83d5381d0df2c316e65ed7
> Author: Jiayuan Chen <jiayuan.chen@linux.dev>
>
> selftests/bpf: Add tests for memory usage for arena
>
> This patch adds a selftest to verify memory usage tracking for BPF arenas.
> It allocates and frees arena pages both from BPF and user-space and checks
> the map's memlock value via fdinfo to ensure it tracks populated pages.
>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/arena_mem_usage.c b/tools/testing/selftests/bpf/prog_tests/arena_mem_usage.c
>> new file mode 100644
>> index 0000000000000..14c2d1a1d673c
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/arena_mem_usage.c
> [ ... ]
>
>> +void serial_test_arena_mem_usage(void)
>> +{
>> +	struct arena_mem_usage *skel;
>> +	const long ps = PAGE_SIZE;
>> +	char *base;
>> +	size_t sz;
>> +	int fd, i;
>> +
>> +	skel = arena_mem_usage__open_and_load();
>> +	if (!ASSERT_OK_PTR(skel, "open_load"))
>> +		return;
>> +	fd = bpf_map__fd(skel->maps.arena);
>> +
>> +	/* Fresh arena: no data pages, and the scratch page is not counted. */
>> +	ASSERT_EQ(map_memlock(fd), 0, "initial");
>> +
>> +	/* BPF-side allocation of 17 pages. */
>> +	skel->bss->alloc_cnt = 17;
>> +	if (run(skel->progs.alloc, "alloc"))
>> +		goto out;
>> +	/*
>> +	 * A NULL ptr means bpf_arena_alloc_pages() itself failed (e.g. the host
>> +	 * is under memory pressure), not a miscount -- flag it distinctly so a
>> +	 * red CI run is not mistaken for a counting bug.
>> +	 */
>> +	if (!ASSERT_OK_PTR(skel->bss->ptr, "arena_alloc_pages"))
> [Severity: Medium]
> Will this check actually catch a NULL pointer when bpf_arena_alloc_pages()
> fails?
>
> When skel->bss->ptr is NULL, ASSERT_OK_PTR() evaluates libbpf_get_error(NULL),
> which returns -errno. Since the userspace errno is likely 0 following a
> successful bpf_prog_test_run_opts() ioctl call, ASSERT_OK_PTR silently
> evaluates to true (success).
>
> This would cause the goto out to be skipped, and the test to proceed to
> evaluate ASSERT_EQ(map_memlock(fd), 17 * ps). This subsequent check would
> then fail, emitting the exact counting bug error the comment mentions trying
> to avoid.
>
> Should this use a different macro, like ASSERT_NEQ(skel->bss->ptr, NULL), to
> correctly verify the userspace pointer?


On success ptr is a valid arena address and ASSERT_OK_PTR() passes 
correctly.

On alloc failure ptr is NULL and ASSERT_OK_PTR() may treat it as OK 
(libbpf_get_error(NULL)
returns -errno, which can be 0), but the next ASSERT_EQ(map_memlock(fd), 
17 * ps) still fails.

I can switch it to ASSERT_NEQ(skel->bss->ptr, NULL, ...) in the next 
revision.