Re: [PATCH bpf-next v2 5/5] selftests/bpf: Cover __uninit kfunc output argument slots

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: ineffective test output_passed_on_stack for __uninit tracking
--

commit 70a455597d07423eca32cc332d11faa394e65772
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

selftests/bpf: Cover __uninit kfunc output argument slots

This commit adds regression tests for the verifier's tracking of __uninit
kfunc output argument slots. It covers cases involving multiple outputs,
outputs following by-value parameters, and output pointers passed on the
stack.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_kfunc_uninit_multi.c b/tools/testing/selftests/bpf/progs/verifier_kfunc_uninit_multi.c
> new file mode 100644
> index 0000000000000..b944346b35e34
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/verifier_kfunc_uninit_multi.c

[ ... ]

> +#if defined(__BPF_FEATURE_STACK_ARGUMENT)
> +SEC("tc")
> +__arch_x86_64 __arch_arm64 __arch_riscv64
> +__success __retval(15)
> +__flag(BPF_F_TEST_STATE_FREQ)
> +__caps_unpriv(CAP_BPF | CAP_NET_ADMIN)
> +__prepare_priv
> +__success_unpriv
> +__naked void output_passed_on_stack(void)
> +{
> +	asm volatile (
> +		"*(u64 *)(r10 - 8) = 0;"
> +		"goto +0;"

[Severity: Medium]
Does pre-initializing the output stack slot here prevent the test from
actually catching __uninit tracking regressions?

If the verifier fails to recognize the __uninit annotation, it treats the
argument as a normal pointer. Because the slot is initialized and kept alive
across the goto +0 checkpoint, the memory is tracked as STACK_ZERO. This
satisfies the BPF_READ requirement in unprivileged mode, allowing the
BPF_WRITE check to clobber the slot to STACK_MISC and verify successfully.

Should this test omit the initialization (similar to how it is done in
output_after_by_value_argument) so that a broken verifier evaluates a
BPF_READ on STACK_INVALID, forcing an expected verification failure in
unprivileged mode?

> +		"r1 = 1;"
> +		"r2 = 2;"
> +		"r3 = 3;"
> +		"r4 = 4;"
> +		"r5 = 5;"
> +		"r6 = r10;"
> +		"r6 += -8;"
> +		"*(u64 *)(r11 - 8) = r6;"
> +		"call %[bpf_kfunc_test_uninit_stack];"
> +		"r0 = *(u32 *)(r10 - 8);"
> +		"exit;"
> +		: : __imm(bpf_kfunc_test_uninit_stack) : __clobber_all);
> +}
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916160821.3157543-1-memxor@gmail.com?part=5