Re: [PATCH bpf-next v2 20/23] selftests/bpf: Add precision backtracking test for stack arguments

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Commit message and code comment incorrectly claim that the subprogram performs a bounds check.
- [Medium] Non-deterministic branching in the test program can lead to intermittent flaky failures by returning 36 instead of the asserted 25.
--

commit 953824567df7ff801d99eb6df49d76d95736325d
Author: Yonghong Song <yonghong.song@linux.dev>

selftests/bpf: Add precision backtracking test for stack arguments

This commit adds a test to verify precision backtracking across BPF-to-BPF
calls involving stack arguments. The test aims to ensure precision propagates
from a kfunc's size argument back through a subprogram frame.

> The test passes a size value as incoming stack arg (arg6) to a
> subprog, which bounds-checks it and forwards it as the mem__sz
> parameter (outgoing arg7) to bpf_kfunc_call_stack_arg_mem.

Is the description about bounds-checking accurate? 

Looking at the inline assembly for subprog_call_mem_kfunc, it appears the
subprogram simply reads the size from the incoming stack slot and immediately
writes it to the outgoing stack slot without any bounds checking logic.

> diff --git a/tools/testing/selftests/bpf/progs/stack_arg_precision.c b/tools/testing/selftests/bpf/progs/stack_arg_precision.c
> new file mode 100644
> index 0000000000000..c94905f07dcc1
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/stack_arg_precision.c
> @@ -0,0 +1,137 @@

[ ... ]

> +/*
> + * Test precision backtracking across bpf-to-bpf call for kfunc stack arg.
> + * subprog_call_mem_kfunc receives a size as incoming stack arg (arg6),
> + * bounds-checks it, then passes it as mem__sz (arg7) to
> + * bpf_kfunc_call_stack_arg_mem.
> + *
> + * 1+2+3+4+5+(1+2+3+4) = 25
> + */

This isn't a bug, but should the comment be updated to match the
implementation? As mentioned above, the bounds check seems to be missing from
the assembly below.

> +__naked __noinline __used
> +static long subprog_call_mem_kfunc(long a, long b, long c, long d, long e, long size)
> +{
> +	asm volatile (
> +		"r1 = *(u64 *)(r11 + 8);"	/* r1 = incoming arg6 (size) */
> +		"r2 = 0x0807060504030201 ll;"	/* r2 = buf contents */
> +		"*(u64 *)(r10 - 8) = r2;"	/* store buf to stack */
> +		"r2 = r10;"
> +		"r2 += -8;"			/* r2 = &buf */
> +		"*(u64 *)(r11 - 8) = r2;"	/* outgoing arg6 = buf */
> +		"*(u64 *)(r11 - 16) = r1;"	/* outgoing arg7 = size */

[ ... ]

> +__naked void stack_arg_precision_bpf2bpf(void)
> +{
> +	asm volatile (
> +		"call %[bpf_get_prandom_u32];"
> +		"r6 = r0;"
> +		"r1 = 1;"
> +		"r2 = 2;"
> +		"r3 = 3;"
> +		"r4 = 4;"
> +		"r5 = 5;"
> +		"if r6 < 2 goto l0_%=;"
> +		"*(u64 *)(r11 - 8) = 4;"
> +		"goto l1_%=;"
> +	"l0_%=:"
> +		"*(u64 *)(r11 - 8) = 6;"
> +	"l1_%=:"
> +		"call subprog_call_mem_kfunc;"
> +		"exit;"
> +		:: __imm(bpf_get_prandom_u32)
> +		: __clobber_all
> +	);
> +}

Could this lead to intermittent test failures?

The test asserts a strict return value of 25:

__success __retval(25)

If bpf_get_prandom_u32() happens to return 0 or 1, the size argument is set to
6 instead of 4. In that case, bpf_kfunc_call_stack_arg_mem() will sum up 6
bytes instead of 4, returning 36 instead of 25, which would cause the test to
fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260507212942.1122000-1-yonghong.song@linux.dev?part=20