bpf: Recognize special arithmetic shift in the verifier

[kernel-patches-daemon-bpf]

Pull request for series with
subject: bpf: Recognize special arithmetic shift in the verifier
version: 2
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1023786

[kernel-patches-daemon-bpf]

Upstream branch: 4f7bc83
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1023786
version: 2

[kernel-patches-daemon-bpf]

Upstream branch: 6cc73f3
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1023786
version: 2

[kernel-patches-review-bot]

commit a5043b2c9e905e8b9fdb9b479b136f5ef5f8415d
Author: Alexei Starovoitov <[email protected]>

selftests/bpf: Add tests for s>>=31 and s>>=63

Add tests for special arithmetic shift right.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_subreg.c b/tools/testing/selftests/bpf/progs/verifier_subreg.c
> index 8613ea160..0b572c067 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_subreg.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_subreg.c
> @@ -670,4 +670,47 @@ __naked void ldx_w_zero_extend_check(void)
>  	: __clobber_all);
>  }
>
> +SEC("socket")
> +__description("s>>=31")
> +__success __success_unpriv __retval(0)
> +__naked void arsh_31(void)
> +{
> +	/* Below is what LLVM generates in cilium's bpf_wiregard.o */
> +	asm volatile ("					\
> +	call %[bpf_get_prandom_u32];			\
> +	w2 = w0;					\
> +	w2 s>>= 31;					\
> +	w2 &= -134; /* w2 becomes 0 or -134 */		\
> +	if w2 s> -1 goto +2;				\
> +	if w2 != -136 goto +1;				\
                   ^^^^

Should this be -134 instead of -136?  The comment on the previous line
states that w2 becomes either 0 or -134 after the AND operation, so w2
can never equal -136.  This means the condition is always true and the
division by zero at "w0 /= 0" is never reachable.

In arsh_31(), after the arithmetic right shift by 31 bits, w2 will be
either 0 (if the sign bit was 0) or -1 (if the sign bit was 1). Then
"w2 &= -134" produces either 0 or -134, never -136.

> +	w0 /= 0;					\
> +	w0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}
> +
> +SEC("socket")
> +__description("s>>=63")
> +__success __success_unpriv __retval(0)
> +__naked void arsh_63(void)
> +{
> +	/* Copy of arsh_31 with s/w/r/ */
> +	asm volatile ("					\
> +	call %[bpf_get_prandom_u32];			\
> +	r2 = r0;					\
> +	r2 <<= 32;					\
> +	r2 s>>= 63;					\
> +	r2 &= -134;					\
> +	if r2 s> -1 goto +2;				\
> +	if r2 != -136 goto +1;				\
                   ^^^^

Same issue here - should this be -134?  After the shift and AND
operations, r2 can only be 0 or -134, making the -136 check always true
and the "r0 /= 0" unreachable.

In arsh_63(), after left-shifting by 32 bits and then arithmetic
right-shifting by 63 bits, r2 becomes either 0 or -1. Then "r2 &= -134"
produces either 0 or -134, never -136.

> +	r0 /= 0;					\
> +	r0 = 0;						\
> +	exit;						\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}
> +
>  char _license[] SEC("license") = "GPL";

---

AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

In-Reply-To-Subject: selftests/bpf: Add tests for s>>=31 and s>>=63
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19383126218

[kernel-patches-daemon-bpf]

Upstream branch: 4722981
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1023786
version: 2

[kernel-patches-daemon-bpf]

Forwarding comment 3535471021 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/