Re: [PATCH bpf v3 2/2] selftests/bpf: Add regression test for kfunc implicit arg injection

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Multi-line comment does not follow the BPF subsystem comment style.
- [Low] Build failure in selftests due to accessing a non-existent BSS field in the reused struct_ops skeleton.
--

commit 70edd2df92a9670c07a307bbdf1081733e04e3dd
Author: Yuan Chen <chenyuan@kylinos.cn>

selftests/bpf: Add regression test for kfunc implicit arg injection

This commit adds a regression test to verify that the BPF verifier correctly
injects the bpf_prog_aux pointer for KF_IMPLICIT_ARGS kfuncs. It uses inline
assembly to contaminate R2 before calling a kfunc to ensure the kernel properly
overwrites it with the valid pointer.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
> index 461ded7223515..fb4d06dd6c4dc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_assoc.c

[ ... ]

> @@ -97,6 +103,7 @@ static void test_st_ops_assoc_reuse(void)
>  
>  	ASSERT_EQ(skel->bss->test_err_a, 0, "skel->bss->test_err_a");
>  	ASSERT_EQ(skel->bss->test_err_b, 0, "skel->bss->test_err_b");
> +	ASSERT_EQ(skel->bss->test_err_inject, 0, "skel->bss->test_err_inject");

[Severity: Low]
Will this cause a build failure? It looks like test_err_inject is defined
in struct_ops_assoc.c, but this function is testing the skeleton for
struct_ops_assoc_reuse.c.

Since the variable isn't defined in the reused program's source, the
generated struct_ops_assoc_reuse skeleton won't contain the test_err_inject
BSS field, which would cause a compiler error when accessing
skel->bss->test_err_inject here.

>  
>  out:
>  	struct_ops_assoc_reuse__destroy(skel);
> diff --git a/tools/testing/selftests/bpf/progs/struct_ops_assoc.c b/tools/testing/selftests/bpf/progs/struct_ops_assoc.c
> index 68842e3f936b9..ed0084453d560 100644
> --- a/tools/testing/selftests/bpf/progs/struct_ops_assoc.c
> +++ b/tools/testing/selftests/bpf/progs/struct_ops_assoc.c
> @@ -103,3 +103,43 @@ SEC(".struct_ops.link")
>  struct bpf_testmod_multi_st_ops st_ops_map_b = {
>  	.test_1 = (void *)test_1_b,
>  };
> +
> +/* Test for aux injection with stale register contamination.
> + *

[Severity: Low]
This isn't a bug, but does this violate the BPF subsystem multi-line
comment style? The guidelines require the opening /* to be on its own
line, rather than starting the text on the same line.

> + * This test verifies that the kernel correctly injects the implicit
> + * bpf_prog_aux pointer for kfuncs with KF_IMPLICIT_ARGS.  The program
> + * uses inline assembly to contaminate R2 with a known magic value
> + * before calling the kfunc:

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