Re: [PATCH RFC bpf-next 3/4] bpf: replace min/max fields with struct cnum{32,64}

On Tue, 2026-04-21 at 18:06 +0000, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [High] Incorrect range constructor used for BPF_JSLT (signed less-than), causing verifier soundness issues.
> --
>
> commit 1fb11732c3d626f4a1777699934aa05d07b7eb56
> Author: Eduard Zingerman <[email protected]>
>
> bpf: replace min/max fields with struct cnum{32,64}
>
> This commit replaces the independent s64, u64, s32, and u32 min/max fields in
> bpf_reg_state with two circular number fields, cnum64 and cnum32. It also
> refactors how bounds deductions are performed and updates conditional branch
> refinements to use circular numbers for set intersections.
>
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 2e896f5d92a2d..90ed32f064650 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
>
> [ ... ]
>
> > @@ -15861,38 +15209,54 @@ static void regs_refine_cond_op(struct bpf_reg_state *reg1, struct bpf_reg_state
>
> [ ... ]
>
> >  	case BPF_JSLT:
> >  		if (is_jmp32) {
> > -			reg_set_srange32(reg1, reg_s32_min(reg1), min(reg_s32_max(reg1), reg_s32_max(reg2) - 1));
> > -			reg_set_srange32(reg2, max(reg_s32_min(reg1) + 1, reg_s32_min(reg2)), reg_s32_max(reg2));
> > +			lo32 = cnum32_from_urange(S32_MIN, reg_s32_max(reg2) - 1);
> > +			hi32 = cnum32_from_urange(reg_s32_min(reg1) + 1, S32_MAX);
> > +			reg1->r32 = cnum32_intersect(reg1->r32, lo32);
> > +			reg2->r32 = cnum32_intersect(reg2->r32, hi32);
> >  		} else {
> > -			reg_set_srange64(reg1, reg_smin(reg1), min(reg_smax(reg1), reg_smax(reg2) - 1));
> > -			reg_set_srange64(reg2, max(reg_smin(reg1) + 1, reg_smin(reg2)), reg_smax(reg2));
> > +			lo = cnum64_from_urange(S64_MIN, reg_smax(reg2) - 1);
> > +			hi = cnum64_from_urange(reg_smin(reg1) + 1, S64_MAX);
> > +			reg1->r64 = cnum64_intersect(reg1->r64, lo);
> > +			reg2->r64 = cnum64_intersect(reg2->r64, hi);
> >  		}
> >  		break;
>
> Are the unsigned range constructors cnum32_from_urange() and
> cnum64_from_urange() correct to use here for a signed less-than comparison?
>
> Passing S32_MIN and S64_MIN into the unsigned constructors might cause them to
> evaluate min > max, resulting in invalid or empty ranges being intersected with
> the registers. Should these use cnum32_from_srange() and cnum64_from_srange()
> like the BPF_JSLE case does?

This is a bug as well.