Re: [PATCH v2 2/2] selftests/bpf: add callback stack-argument rejection test

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added selftest lacks a `__msg(...)` annotation and the verifier silently rejects the program, rendering the test ineffective at preventing regressions of the vulnerability.
--

--- Patch [2]: [PATCH v2 2/2] selftests/bpf: add callback stack-argument rejection test ---
commit 3ed3bf88f0e2a4dcd1bfa96fc2ece2b85d98465e
Author: Jérémy Jean <[email protected]>

selftests/bpf: add callback stack-argument rejection test

This commit adds verifier coverage for callback subprograms that declare
incoming stack arguments. It passes a nine-argument callback to bpf_loop()
and expects the verifier to reject it.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> index 7e0ce5db28a0d..50b276fc2c3f2 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
[ ... ]
> @@ -79,6 +86,31 @@ __naked void stack_arg_two_subprogs(void)
>  	);
>  }
>  
> +SEC("tc")
> +__description("stack_arg: callback with incoming stack args")
> +__failure

[Severity: Medium]
Should this test include a __msg(...) annotation to verify the exact rejection
reason?

Because the test manually prepares outgoing stack slots (*(u64 *)(r11 - 32) = 0;
etc.), it might fall back to failing in bpf_fixup_call_args() if the primary
callback check in push_callback_call() regresses. This would cause the test to
falsely report success even if the intended security check is bypassed.

Additionally, push_callback_call() in the verifier currently rejects the
callback by silently returning -EINVAL without emitting a verbose() error
message. Does the verifier need to be updated to emit a log message so that
this selftest can properly assert against it?

> +__naked void stack_arg_callback_many_args(void)
> +{
> +	asm volatile (
> +		"r6 = 0;"
> +		"*(u64 *)(r11 - 32) = 0;"
> +		"*(u64 *)(r11 - 24) = 0;"
> +		"*(u64 *)(r11 - 16) = 0;"
> +		"*(u64 *)(r11 - 8) = 0;"
> +		"r1 = 1;"
> +		"r2 = %[callback_9args];"
> +		"r3 = 0;"
> +		"r4 = 0;"
> +		"call %[bpf_loop];"
> +		"r0 = 0;"
> +		"exit;"
> +		:
> +		: __imm_ptr(callback_9args),
> +		  __imm(bpf_loop)
> +		: __clobber_common, "r6"
> +	);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2