Re: [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer zext across state pruning

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The selftest uses a 64-bit instruction (`r6 = 0`) instead of a 32-bit instruction (`w6 = 0`), which zeroes the upper 32 bits and bypasses `BPF_F_TEST_RND_HI32` randomization, masking the bug it intends to catch.
- [Medium] The test's exit logic masks wild pointer faults via BPF exception tables, causing it to always return 0 (success) even if it successfully reproduces the bug.
--

--- Patch [4]: [PATCH bpf-next 4/4] selftests/bpf: Add test for arena pointer zext across state pruning ---
commit 230d6f79c5ec9b66b6dcd9f9c3cb999533024b62
Author: Daniel Borkmann <[email protected]>

selftests/bpf: Add test for arena pointer zext across state pruning

This patch adds a selftest to verify that the BPF verifier correctly marks
zero extension for arena pointers across state pruning. The test uses an
addr_space_cast and relies on BPF_F_TEST_RND_HI32 to ensure proper zero
extension on architectures like s390x.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_arena.c b/tools/testing/selftests/bpf/progs/verifier_arena.c
> index b241bbcf54a8a..b5f5b81a27e71 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_arena.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_arena.c
[ ... ]
> @@ -635,6 +635,41 @@ int non_arena_ptr_add_to_arena_ptr(void *ctx)
[ ... ]
> +	if r7 != 0 goto l1_%=;				\
> +	goto l0_%=;					\
> +l1_%=:	r6 = 0;			/* 32-bit define */	\

[Severity: Medium]
Does using a 64-bit assignment here bypass the BPF_F_TEST_RND_HI32
randomization framework?

Since r6 = 0 translates to a BPF_ALU64 instruction, bpf_is_reg64() returns
true and the verifier's bpf_opt_subreg_zext_lo32_rnd_hi32() pass skips
instrumenting it. This prevents garbage from being injected into the upper 32
bits, masking the missing zero-extension bug the test intends to catch.

Would using a 32-bit assignment like w6 = 0 correctly trigger the
randomization?

> +	r6 = addr_space_cast(r6, 0x0, 0x1);		\
> +l0_%=:	r0 = *(u32 *)(r6 + 0);	/* 64-bit read */	\
> +	r0 = 0;						\
> +	exit;						\

[Severity: Medium]
Will this exit logic mask wild pointer faults?

Because r6 is a PTR_TO_ARENA, the verifier converts the out-of-bounds read
into a BPF_PROBE_MEM32 load. If r6 contains a wild garbage pointer and triggers
a page fault, the kernel's exception tables will silently catch it, zero r0,
and resume execution.

Since the test then explicitly executes r0 = 0 and exits, and the framework
expects __retval(0), it appears the test will unconditionally pass even if it
successfully reproduces the bug.

> +"	:
> +	: __imm_addr(arena),
> +	  __imm_const(__sk_buff_len, offsetof(struct __sk_buff, len))
> +	: __clobber_all);
> +}

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