Re: [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: selftests fail to verify bounds tracking of rx_queue_mapping
--

commit 90d4583595dcbfcce26a9758d799968a757439e5
Author: Emil Tsalapatis <emil@etsalapatis.com>

selftests/bpf: Add selftests for rx_queue_mapping context access

This commit adds selftests for the rx_queue_mapping field read from
struct sock to ensure the verifier properly tracks its 0 bit state and
width.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
> index 2a136c917680f..565c4d8bc3fa3 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_sock.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c

[ ... ]

> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [no sign extension]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_no_sign_ext(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64*)(r1 + %[__sk_buff_sk]);		\
> +	if r1 != 0 goto l0_%=;				\
> +	r0 = 0xdead;					\
> +	exit;						\
> +l0_%=:	r0 = *(u32*)(r1 + %[bpf_sock_rx_queue_mapping]);	\
> +	r0 >>= 32;					\
> +	exit;						\

[Severity: Medium]
Does this sequence actually verify the verifier's 0-bit state tracking
of rx_queue_mapping?

The program shifts the loaded value right by 32 and exits. The verifier
always accepts returning an unknown initialized scalar in this context. At
runtime, the CPU zero-extends the value, so it always evaluates to 0 and
passes the __retval(0) check, regardless of what the verifier's static
analysis concluded about the bounds.

> +"	:
> +	: __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> +	  __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> +	: __clobber_all);
> +}
> +
> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [narrow load mask]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_narrow_load_mask(void)
> +{
> +	asm volatile ("					\
> +	r1 = *(u64*)(r1 + %[__sk_buff_sk]);		\
> +	if r1 != 0 goto l0_%=;				\
> +	r0 = 0xdead;					\
> +	exit;						\
> +l0_%=:	r0 = *(u16*)(r1 + %[bpf_sock_rx_queue_mapping]);	\
> +	r0 >>= 16;					\
> +	exit;						\

[Severity: Medium]
Similarly, does this narrow load test pass at runtime without strictly
asserting the verifier's internal bounds tracking?

Like the previous function, this shifts the 16-bit load right by 16 and
exits. It seems this will also implicitly pass at runtime rather than
causing a verification failure if bounds tracking regressions occur.

> +"	:
> +	: __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> +	  __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> +	: __clobber_all);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922172028.6269-1-emil@etsalapatis.com?part=4