Re: [PATCH bpf v5 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:
- [Medium] Unsafe memory access in `bpf_kfunc_implicit_arg` leading to kernel panic on verifier injection failure
- [Low] BPF subsystem multi-line comment style violation
--

commit 7fe0e7a112c19898e04e79eb1d5d25a017d9fcd2
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 kernel correctly
injected the bpf_prog_aux pointer. It checks the name field of the aux pointer
to ensure it matches the expected prefix, attempting to catch cases where the
verifier leaves a stale register value.

> 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 preferred BPF
subsystem comment style?

The BPF subsystem guidelines require multi-line comments to have the
opening /* on its own line:

/*
 * Verify the kernel injected the correct bpf_prog_aux pointer
 * rather than leaving a stale register value.
 */

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

[Severity: Medium]
If the verifier skips injection and leaves a stale, non-NULL register
value in the aux variable, won't dereferencing aux->name cause a kernel
panic?

The commit message mentions that in cases like a BTF mismatch, the stale
register would not contain a valid aux pointer. If aux is a garbage pointer
rather than NULL, the !aux check will evaluate to false, and the strncmp
call will attempt to read from unmapped or invalid memory.

Could this use something like copy_from_kernel_nofault() or similar safe
memory access to gracefully fail the test instead of crashing the host kernel?

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

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