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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The test logic unintentionally bypasses the explicit check for allocation failure, defeating its documented purpose to distinctly flag memory pressure CI failures.
--

--- Patch [3]: [PATCH bpf-next 3/3] selftests/bpf: Add tests for memory usage for arena ---
commit f5d331845302b77cab3c4b06daa82ffb16675b30
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

selftests/bpf: Add tests for memory usage for arena

This commit adds tests to verify that memory usage for BPF arenas is tracked
correctly in the map's fdinfo memlock field. It allocates and frees arena
pages from both BPF and user-space fault-in paths.

> 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

[ ... ]

> +static int run(struct bpf_program *prog, const char *name)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, opts);
> +	int err = bpf_prog_test_run_opts(bpf_program__fd(prog), &opts);
> +
> +	if (!ASSERT_OK(err, name))
> +		return -1;
> +	if (!ASSERT_OK(opts.retval, name))
> +		return -1;
> +	return 0;
> +}

[ ... ]

> +void serial_test_arena_mem_usage(void)
> +{

[ ... ]

> +	/* 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"))
> +		goto out;

[Severity: Medium]
Does this logic inadvertently bypass the explicit check for allocation failure?

Looking at the alloc() BPF program in arena_mem_usage.c, it returns 1 on
failure:

SEC("syscall")
int alloc(void *ctx)
{
        ptr = bpf_arena_alloc_pages(&arena, NULL, alloc_cnt, NUMA_NO_NODE, 0);
        return ptr ? 0 : 1;
}

Because of this, if bpf_arena_alloc_pages() fails, opts.retval will be 1,
causing run() to assert and return -1.

The test will then take the "goto out" path immediately after run(), making
the ASSERT_OK_PTR() check unreachable and defeating its documented purpose
to distinctly flag memory pressure.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716142746.8794-1-jiayuan.chen@linux.dev?part=3