Re: [PATCH bpf-next v2 10/13] selftests/bpf: cover the low-32 link for narrowing stack fills

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly added tests for narrow stack fills hardcode memory offsets (e.g., `(r10 - 8)`) which are only correct on little-endian architectures. On big-endian architectures like s390x, these offsets do not point to the lower bits, causing the BPF verifier to reject the programs and resulting in test failures.
--

commit a42f9a6d4d49f17385f014eede55d6e82094a838
Author: Vineet Gupta <vineet.gupta@linux.dev>

selftests/bpf: cover the low-32 link for narrowing stack fills

This commit adds five new BPF programs to test the verifier's handling of
narrowing stack fills, specifically targeting low-32 links and sign-extension
behavior.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
> --- a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c

[ ... ]

> +SEC("socket")
> +__success
> +__naked void zext_fill_narrow_from_wide_spill(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;						\
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;						\
> +	r6 |= r0;		/* r6 = full 64-bit unknown */	\
> +	*(u64 *)(r10 - 8) = r6;	/* slot linked to r6 */		\
> +	r2 = *(u32 *)(r10 - 8);	/* narrowing fill, forms the link */ \

[Severity: Medium]
On big-endian architectures like s390x, won't reading 32 bits from offset -8
target the upper 32 bits instead of the lower 32 bits?

The bpf_stack_narrow_access_ok() check in the verifier expects to read the
least significant bytes of the spill slot, which on big-endian are located
at offset -4 for a 32-bit read. Could this hardcoded offset cause the verifier
to reject the program and break the test on big-endian systems?

> +	if w6 != 0 goto 1f;	/* narrows r6, propagates to r2 */ \
> +	if r2 == 0 goto 1f;					\
> +	r0 /= 0;						\
> +1:								\
> +	r0 = 0;							\
> +	exit;							\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

[ ... ]

> +SEC("socket")
> +__failure __msg("div by zero")
> +__naked void zext_fill_byte_forms_no_link(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;						\
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;						\
> +	r6 |= r0;						\
> +	*(u64 *)(r10 - 8) = r6;					\
> +	r2 = *(u8 *)(r10 - 8);	/* 1-byte fill: no link */	\

[Severity: Medium]
Does this hardcoded 8-bit read from offset -8 target the most significant byte
rather than the least significant byte on big-endian architectures?

> +	if w6 != 0 goto 1f;					\
> +	if r2 == 0 goto 1f;	/* not deduced */		\
> +	r0 /= 0;						\
> +1:								\
> +	r0 = 0;							\
> +	exit;							\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

[ ... ]

> +SEC("socket")
> +__success
> +__naked void sext_fill_narrow_from_wide_spill(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;						\
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;						\
> +	r6 |= r0;		/* r6 = full 64-bit unknown */	\
> +	*(u64 *)(r10 - 8) = r6;	/* slot linked to r6 */		\
> +	r2 = *(s32 *)(r10 - 8);	/* narrowing sx fill */		\

[Severity: Medium]
Similar to the previous cases, will this 32-bit read from offset -8 read the
upper 32 bits on big-endian systems and cause verifier bounds tracking to fail?

> +	if w6 != -1 goto 1f;	/* narrows r6, propagates to r2 */ \
> +	if r2 == -1 goto 1f;	/* sign-extended, not 0xffffffff */ \
> +	r0 /= 0;						\
> +1:								\
> +	r0 = 0;							\
> +	exit;							\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

[ ... ]

> +SEC("socket")
> +__failure __msg("div by zero")
> +__flag(BPF_F_TEST_STATE_FREQ)
> +__naked void fill_kinds_reach_different_values(void)
> +{
> +	asm volatile ("						\
> +	call %[bpf_get_prandom_u32];				\
> +	r6 = r0;						\
> +	r6 &= 1;						\
> +	call %[bpf_get_prandom_u32];				\
> +	r8 = r0;						\
> +	call %[bpf_get_prandom_u32];				\
> +	r0 <<= 32;						\
> +	r8 |= r0;						\
> +	*(u64 *)(r10 - 8) = r8;					\
> +	if r6 >= 1 goto 2f;					\
> +	r2 = *(s32 *)(r10 - 8);	/* sign-extending fill */	\

[Severity: Medium]
Will these narrow fills also fail verifier checks on big-endian machines due
to the fixed -8 offset pointing to the wrong 32-bit half?

> +	goto 1f;						\
> +2:								\
> +	r2 = *(u32 *)(r10 - 8);	/* zero-extending fill */	\
> +1:								\
> +	if w8 != -1 goto 3f;					\
> +	if r2 == -1 goto 3f;	/* only the sign-extending path */ \

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260910164635.459558-1-vineet.gupta@linux.dev?part=10