Re: [PATCH bpf-next 3/4] selftests/bpf: Test cross-sign 64bits range refinement

[Eduard Zingerman <eddyz87@gmail.com>]

On Sat, 2025-07-19 at 16:23 +0200, Paul Chaignon wrote:

[...]

> The first patch in this series ("bpf: Improve bounds when s64 crosses
> sign boundary") fixes this by refining ranges before we reach the
> condition, such that the verifier can detect the jump is always taken.
> Indeed, at instruction 7, the ranges look as follows:
> 
>     0          umin=0xfffffcf1                 umax=0xff..ff6e  U64_MAX
>     |                [xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx]        |
>     |----------------------------|------------------------------|
>     |xxxxxxxxxx]                                   [xxxxxxxxxxxx|
>     0    smax=0xeffffeee                       smin=-655        -1

I'd move this diagram to the selftest itself.

> 
> The updated __reg64_deduce_bounds can therefore improve the ranges to
> s64=[-655; -146] (and the u64 equivalent). With this new range, it's
> clear that the condition at instruction 8 is always true: R3's umax is
> 0xffffffff and R0's umin is 0xfffffffffffffd71 ((u64)-655). We avoid the
> dead branch and don't end up with an invariant violation.
> 
> Link: https://syzkaller.appspot.com/bug?extid=c711ce17dd78e5d4fdcf [1]
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
>  .../selftests/bpf/progs/verifier_bounds.c     | 23 +++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/progs/verifier_bounds.c b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> index 63b533ca4933..d104d43ff911 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_bounds.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_bounds.c
> @@ -1550,4 +1550,27 @@ l0_%=:	r0 = 0;				\
>  	: __clobber_all);
>  }
>  
> +SEC("socket")
> +__description("bounds deduction sync cross sign boundary")
> +__success __log_level(2) __flag(BPF_F_TEST_REG_INVARIANTS)
> +__retval(0)
> +__naked void test_invariants(void)

Could you please check deduced range with __msg?

> +{
> +	asm volatile("			\
> +	call %[bpf_get_prandom_u32];	\
> +	w3 = w0;			\
> +	w6 = (s8)w0;			\
> +	r0 = (s8)r0;			\
> +	if w6 >= 0xf0000000 goto l0_%=;	\
> +	r0 += r6;			\
> +	r6 += 400;			\
> +	r0 -= r6;			\
> +	if r3 < r0 goto l0_%=;		\
> +l0_%=:	r0 = 0;				\
> +	exit;				\
> +"	:
> +	: __imm(bpf_get_prandom_u32)
> +	: __clobber_all);
> +}

I think two more test cases are needed:
- when intersection is on the other side of the interval;
- when signed and unsigned intervals overlap in two places.