Re: [PATCH bpf-next v6 17/17] selftests/bpf: Add verifier tests for stack argument validation
Yonghong Song <[email protected]>
| Newsgroups | dev.linux.lists.sashiko,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 4/19/26 10:21 AM, [email protected] wrote: > Sashiko AI review found 1 potential issue(s): > - [Medium] Test fragility in `subprog_deref_arg6` error matching due to compiler register allocation > -- > > commit 3936b0a6dea45627278be80c25b20ff18f2821d4 > Author: Yonghong Song <[email protected]> > > selftests/bpf: Add verifier tests for stack argument validation > > This commit adds inline-asm based verifier tests to exercise BPF stack > argument validation logic directly, including both positive and negative > tests for tracking pointers, references, and null propagation. > >> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c >> new file mode 100644 >> index 0000000000000..66dd11840a63e >> --- /dev/null >> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c > [ ... ] > >> +__noinline __used >> +static long subprog_deref_arg6(long a, long b, long c, long d, long e, long *f) >> +{ >> + return *f; >> +} > [ ... ] > >> +SEC("tc") >> +__description("stack_arg: pruning with different stack arg types") >> +__failure >> +__flag(BPF_F_TEST_STATE_FREQ) >> +__arch_x86_64 >> +__msg("R1 invalid mem access 'scalar'") > Does this hardcode an assumption about Clang's register allocation? Yes, it is possible that clang may pick a different register. So as you suggested, I will use regex like below: __msg("R{{[0-9]}} invalid mem access 'scalar'") which should fix the issue. > > Since subprog_deref_arg6 is standard C rather than a __naked inline > assembly function, the compiler determines which register is used to > dereference f. If a future LLVM version chooses a different register, the > tests will fail because the verifier error will mention the new register > instead of R1. > > Could these tests use __regex to match the register dynamically, or be > rewritten so subprog_deref_arg6 is a __naked function to guarantee the > register choice? > >> +__naked void stack_arg_pruning_type_mismatch(void) >> +{ >> + asm volatile ( >> + "call %[bpf_get_prandom_u32];" > [ ... ] >