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

[sashiko-bot@kernel.org]

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 <eddyz87@gmail.com>

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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421-cnums-everywhere-rfc-v1-v1-0-8f8e98537f48@gmail.com?part=3