Re: [PATCH bpf-next v3 20/24] selftests/bpf: Add verifier tests for stack argument validation

[Yonghong Song <yonghong.song@linux.dev>]

On 5/11/26 8:19 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/prog_tests/verifier.c b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> index a96b25ebff23..ee3d929fac8a 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/verifier.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/verifier.c
>> @@ -91,6 +91,8 @@
>>   #include "verifier_sockmap_mutate.skel.h"
>>   #include "verifier_spill_fill.skel.h"
>>   #include "verifier_spin_lock.skel.h"
>> +#include "verifier_stack_arg.skel.h"
>> +#include "verifier_stack_arg_order.skel.h"
>>   #include "verifier_stack_ptr.skel.h"
>>   #include "verifier_store_release.skel.h"
>>   #include "verifier_subprog_precision.skel.h"
>> @@ -238,6 +240,8 @@ void test_verifier_sock_addr(void)            { RUN(verifier_sock_addr); }
>>   void test_verifier_sockmap_mutate(void)       { RUN(verifier_sockmap_mutate); }
>>   void test_verifier_spill_fill(void)           { RUN(verifier_spill_fill); }
>>   void test_verifier_spin_lock(void)            { RUN(verifier_spin_lock); }
>> +void test_verifier_stack_arg(void)            { RUN(verifier_stack_arg); }
>> +void test_verifier_stack_arg_order(void)      { RUN(verifier_stack_arg_order); }
>>   void test_verifier_stack_ptr(void)            { RUN(verifier_stack_ptr); }
>>   void test_verifier_store_release(void)        { RUN(verifier_store_release); }
>>   void test_verifier_subprog_precision(void)    { RUN(verifier_subprog_precision); }
> In v2 (May 7), sashiko-bot raised a concern about whether the tests in
> verifier_stack_arg_order.c are actually being executed:
>
>    The tests in verifier_stack_arg_order.c are never executed because
>    the file is not registered in the selftests runner. [...] Did you
>    intend to include verifier_stack_arg_order.skel.h here as well?
>    [...] Could this be missing a RUN(verifier_stack_arg_order) entry?
>
> Reference: https://lore.kernel.org/bpf/20260509013809.18E7DC2BCB0@smtp.kernel.org/
>
> You responded in v3 (May 10) saying "Will fix," but this commit includes
> both verifier_stack_arg_order.skel.h and RUN(verifier_stack_arg_order),
> which appears to address the concern. Was the registration added after the
> discussion, or is there still something missing?

The new code is added after the discussion. So it is covered.

>
> [ ... ]
>
>> 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 000000000000..d38beba6b5e9
>> --- /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)
>> +__msg("R{{[0-9]}} invalid mem access 'scalar'")
>> +__naked void stack_arg_pruning_type_mismatch(void)
>> +{
>> +	asm volatile (
>> +		"call %[bpf_get_prandom_u32];"
>> +		"r6 = r0;"
>> +		/* local = 0 on program stack */
>> +		"r7 = 0;"
>> +		"*(u64 *)(r10 - 8) = r7;"
>> +		/* Branch based on random value */
>> +		"if r6 s> 3 goto l0_%=;"
>> +		/* Path 1: store stack pointer to outgoing arg6 */
>> +		"r1 = r10;"
>> +		"r1 += -8;"
>> +		"*(u64 *)(r11 - 8) = r1;"
>> +		"goto l1_%=;"
>> +	"l0_%=:"
>> +		/* Path 2: store scalar to outgoing arg6 */
>> +		"*(u64 *)(r11 - 8) = 42;"
>> +	"l1_%=:"
>> +		/* Call subprog that dereferences arg6 */
>> +		"r1 = r6;"
>> +		"r2 = 0;"
>> +		"r3 = 0;"
>> +		"r4 = 0;"
>> +		"r5 = 0;"
>> +		"call subprog_deref_arg6;"
>> +		"exit;"
>> +		:: __imm(bpf_get_prandom_u32)
>> +		: __clobber_all
>> +	);
>> +}
> [ ... ]
>
>> +SEC("tc")
>> +__description("stack_arg: release_reference invalidates stack arg slot")
>> +__failure
>> +__msg("R{{[0-9]}} invalid mem access 'scalar'")
>> +__naked void stack_arg_release_ref(void)
> [ ... ]
>
>> +SEC("tc")
>> +__description("stack_arg: pkt pointer in stack arg slot invalidated after pull_data")
>> +__failure
>> +__msg("R{{[0-9]}} invalid mem access 'scalar'")
>> +__naked void stack_arg_stale_pkt_ptr(void)
> [ ... ]
>
>> +SEC("tc")
>> +__description("stack_arg: null propagation rejects deref on null branch")
>> +__failure
>> +__msg("R{{[0-9]}} invalid mem access 'scalar'")
>> +__naked void stack_arg_null_propagation_fail(void)
> Multiple reviewers (sashiko-bot in v6, bpf-ci bot in April 24 and v2)
> raised concerns about test fragility in these tests
> (stack_arg_pruning_type_mismatch, stack_arg_release_ref,
> stack_arg_stale_pkt_ptr, and stack_arg_null_propagation_fail).
>
> Since subprog_deref_arg6 is standard C (not __naked), LLVM determines which
> register is used for dereferencing. The reviewers noted that if a future
> LLVM version chooses a different register, the verifier error will mention
> that register instead, causing the test to fail.
>
> Reference: https://lore.kernel.org/bpf/b03ce38af2e3d574b5a79963da1a4db70e896e6712b2bf2e9fce69178433540d@mail.kernel.org/
>
> You responded in v6 that you would use a regex pattern like
> R{{[0-9]}}, and in v2 (May 7) you stated you prefer to remove the register
> prefix entirely. The current tests use __msg("R{{[0-9]}} invalid mem access
> 'scalar'"), which matches any register number.
>
> Is the regex pattern R{{[0-9]}} sufficient to address the fragility
> concern, or would it be safer to make subprog_deref_arg6 a __naked
> function with explicit register usage?

I think R{{[0-9]}} is sufficient.

>
> [ ... ]
>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/25652623893