Re: [PATCH bpf-next v5 10/14] selftests/bpf: Add struct_ops __arena and __arena__nullable argument tests

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsafe unconditional dereference of `st_ops3` and its callbacks in the newly added kfuncs, leading to a kernel panic.
--

--- Patch [10]: [PATCH bpf-next v5 10/14] selftests/bpf: Add struct_ops __arena and __arena__nullable argument tests ---
commit ca58e222e16e38d0927f0dd75956c9489242a175
Author: Tejun Heo <[email protected]>

selftests/bpf: Add struct_ops __arena and __arena__nullable argument tests

This patch adds test_arena and test_arena_nullable members to bpf_testmod_ops3
with arena-tagged arguments. It also introduces kfuncs that forward caller-
provided pointers to them to test end-to-end conversions of arena pointers.

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> index 2291bb4665178..1e6d632c6f836 100644
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
> @@ -406,6 +418,16 @@ __bpf_kfunc void bpf_testmod_ops3_call_test_2(void)
>  	st_ops3->test_2();
>  }
>  
> +__bpf_kfunc int bpf_testmod_ops3_call_test_arena(u64 *ptr__arena)
> +{
> +	return st_ops3->test_arena(ptr__arena);

[Severity: High]
Does this code risk a NULL pointer dereference if st_ops3 has not been set yet?

The global pointer st_ops3 is initialized to NULL and only set when a struct_ops
map is explicitly registered. A BPF program could trigger a kernel panic by
invoking this kfunc via BPF_PROG_RUN before the map is attached, or concurrently
during teardown.

While this pattern is also used in the pre-existing bpf_testmod_ops3_call_test_1()
kfunc, should st_ops3 be checked for NULL, potentially while holding
st_ops_mutex, before dereferencing it here?

> +}
> +
> +__bpf_kfunc int bpf_testmod_ops3_call_test_arena_nullable(u64 *ptr__arena__nullable)
> +{
> +	return st_ops3->test_arena_nullable(ptr__arena__nullable);

[Severity: High]
Similarly, can this unconditionally dereference st_ops3 when it is NULL?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10