Re: [PATCH bpf v6 2/2] selftests/bpf: strengthen bpf_kfunc_implicit_arg to verify aux injection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Dereferencing a potentially stale or uninitialized `aux` pointer in `strncmp` will trigger a kernel panic (Oops) instead of gracefully returning a test failure.
- [Low] The multi-line comment does not follow the required BPF subsystem style.
--

commit 9fe1fbc629675a37bbb1c47b4e9b422cd2edf717
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: strengthen bpf_kfunc_implicit_arg to verify aux injection

This commit updates bpf_kfunc_implicit_arg() to verify that the
KF_IMPLICIT_ARGS injection path correctly passes the bpf_prog_aux pointer.
It does this by comparing the name field of the aux pointer to ensure it
matches the expected prefix.

> diff --git a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> --- a/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
> +++ b/tools/testing/selftests/bpf/test_kmods/bpf_testmod.c
[ ... ]
> @@ -1827,7 +1827,11 @@ int bpf_kfunc_multi_st_ops_test_1_assoc(struct st_ops_args *args, struct bpf_pro
>  
>  int bpf_kfunc_implicit_arg(int a, struct bpf_prog_aux *aux)
>  {
> -	if (aux && a > 0)
> +	/* Verify the kernel injected the correct bpf_prog_aux pointer
> +	 * rather than leaving a stale register value. */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the expected
BPF subsystem style? The preferred kernel style typically places the opening
/* on its own line before the text begins.

> +	if (!aux || strncmp(aux->name, "test_kfunc", sizeof("test_kfunc") - 1))

[Severity: High]
If the verifier incorrectly skips injection and leaves a stale register
value in the aux pointer, as described in the commit message, wouldn't calling
strncmp() on aux->name trigger a kernel page fault?

Since the intention here is to catch a stale register containing an arbitrary
unmapped or uninitialized memory address, can we safely dereference it directly
in strncmp() without using a fault-safe memory access method?

> +		return -EINVAL;
> +	if (a > 0)
>  		return a;
>  	return -EINVAL;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609125214.3096351-1-chenyuan_fl@163.com?part=2