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@xxxxxxxxx>
> ---
> .../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.
