On Fri, 2026-08-14 at 16:19 -0700, Vineet Gupta wrote:

...

> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index ef71999c4695..6aaedde6e9d1 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c

...

> @@ -570,9 +572,18 @@ static bool regsafe(struct bpf_verifier_env *env, struct 
> bpf_reg_state *rold,
>                * on a path that predates this series, which is a pruning 
> change
>                * that wants measuring on its own; it is deliberately left
>                * alone here.
> +              *
> +              * Only demand a match when the old state carries a link at all.
> +              * These flags are only ever set together with an ->id, so
> +              * rold->id == 0 implies none is set, and the only case this
> +              * admits is "old knows no low-32 relationship, cur does" -- cur
> +              * is then strictly more constrained than old, which is the safe
> +              * direction for pruning. The reverse is still rejected. Without
> +              * this a register that first acquires a link inside a loop 
> would
> +              * never match its pre-loop state and pruning would not 
> converge.
>                */
>               if (rold->id &&
> -                 (rold->flags & BPF_FLAG_SUBREG_ZEXT) != (rcur->flags & 
> BPF_FLAG_SUBREG_ZEXT))
> +                 (rold->flags & BPF_FLAG_SUBREG) != (rcur->flags & 
> BPF_FLAG_SUBREG))
>                       return false;

Same comment as for ZEXT patch.

>  
>               if (env->explore_alu_limits) {
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8a802d49d0a4..45cb67dc3999 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -14976,6 +14976,8 @@ static int adjust_reg_min_max_vals(struct 
> bpf_verifier_env *env,
>       return 0;
>  }
>  
> +static void reconstruct_sext32(struct bpf_reg_state *reg, struct 
> bpf_reg_state *src);

Nit: is it possible to avoid forward declaration?

> +
>  /* check validity of 32-bit and 64-bit arithmetic operations */
>  static int check_alu_op(struct bpf_verifier_env *env, struct bpf_insn *insn)
>  {
> @@ -15052,15 +15054,65 @@ static int check_alu_op(struct bpf_verifier_env 
> *env, struct bpf_insn *insn)
>                                                       insn->src_reg);
>                                               return -EACCES;
>                                       } else if (src_reg->type == 
> SCALAR_VALUE) {
> +                                             int sz = insn->off >> 3;
>                                               bool no_sext;
> +                                             bool subreg_link;
>  
>                                               no_sext = reg_umax(src_reg) < 
> (1ULL << (insn->off - 1));
> -                                             if (no_sext)
> +                                             /*
> +                                              * When no_sext, dst == src 
> exactly, so link them
> +                                              * (existing behavior). When 
> !no_sext for a 32-bit sign
> +                                              * extension the low 32 bits 
> are still identical (sext
> +                                              * preserves them), so form a 
> BPF_FLAG_SUBREG_SEXT
> +                                              * link: a later narrowing of 
> the low 32 bits
> +                                              * propagates here, and 
> sync_linked_regs() rebuilds
> +                                              * the high half via 
> reconstruct_sext32().
> +                                              *
> +                                              * An ADD_CONST-linked src is 
> excluded for the same
> +                                              * reason as in the 
> zero-extending arm below:
> +                                              * 
> assign_scalar_id_before_mov() would clear its
> +                                              * base+delta link, and a 
> combined subreg+delta link
> +                                              * isn't modeled anyway. Unlike 
> that arm a self-mov is
> +                                              * NOT excluded -- r0 = (s32)r0 
> is the case this is
> +                                              * here for.
> +                                              */
> +                                             subreg_link = (sz == 4) &&
> +                                                           !(src_reg->flags 
> & BPF_FLAG_ADD_CONST);
> +
> +                                             if (no_sext || subreg_link)
>                                                       
> assign_scalar_id_before_mov(env, src_reg);
>                                               *dst_reg = *src_reg;
> -                                             if (!no_sext)
> -                                                     
> clear_scalar_id(dst_reg);
> -                                             coerce_reg_to_size_sx(dst_reg, 
> insn->off >> 3);
> +                                             if (!no_sext) {
> +                                                     if (subreg_link && 
> src_reg->id) {
> +                                                             /* ->id already 
> copied above */
> +                                                             dst_reg->flags 
> = (dst_reg->flags & ~BPF_FLAG_SUBREG) |
> +                                                                             
>  BPF_FLAG_SUBREG_SEXT;
> +                                                     } else {
> +                                                             
> clear_scalar_id(dst_reg);
> +                                                     }
> +                                             }
> +                                             /*
> +                                              * coerce_reg_to_size_sx() 
> falls back to the full sext
> +                                              * range when smin/smax 
> straddle the sign boundary (e.g.
> +                                              * an errno-or-zero value 
> clamped to [-4095, 0]). For a
> +                                              * register tracked as the 
> sign-extension of its low 32
> +                                              * bits the high half IS that 
> sign-extension, so rebuild
> +                                              * the tighter 64-bit range 
> from the low bounds, taken
> +                                              * from a snapshot because 
> coerce overwrites them.
> +                                              *
> +                                              * Gated on sz == 4, not on the 
> flag alone: an (s8)/(s16)
> +                                              * mov whose src is already 
> SEXT-linked copies the flag
> +                                              * across in the *dst_reg = 
> *src_reg above, and a 32-bit
> +                                              * reconstruction must not run 
> for a narrower operation.
> +                                              */
> +                                             if (sz == 4 && (dst_reg->flags 
> & BPF_FLAG_SUBREG_SEXT)) {
> +                                                     struct bpf_reg_state 
> sext_src = *dst_reg;
> +
> +                                                     
> coerce_reg_to_size_sx(dst_reg, sz);
> +                                                     
> reconstruct_sext32(dst_reg, &sext_src);

It does not make sense to maintain two functions that do register sign
extension. If coerce_reg_to_size_sx() is not precise enough for the
32-bit case, then it should be adapted instead of special-cased.

> +                                             } else {
> +                                                     
> coerce_reg_to_size_sx(dst_reg, sz);
> +                                             }
>                                       } else {
>                                               mark_reg_unknown(env, regs, 
> insn->dst_reg);
>                                       }
> @@ -15107,7 +15159,15 @@ static int check_alu_op(struct bpf_verifier_env 
> *env, struct bpf_insn *insn)
>                                               if (!is_src_reg_u32) {
>                                                       if (wide_subreg_link && 
> src_reg->id) {
>                                                               /* ->id already 
> copied above */
> -                                                             dst_reg->flags 
> |= BPF_FLAG_SUBREG_ZEXT;
> +                                                             /*
> +                                                              * 
> Zero-extension: high bits are 0, not a
> +                                                              * 
> sign-extension of the low field. Drop any
> +                                                              * SUBREG_SEXT 
> copied from a sext-linked src
> +                                                              * so 
> sync_linked_regs() rebuilds dst by
> +                                                              * 
> zero-extension, not reconstruct_sext32().
> +                                                              */
> +                                                             dst_reg->flags 
> = (dst_reg->flags & ~BPF_FLAG_SUBREG) |
> +                                                                             
>  BPF_FLAG_SUBREG_ZEXT;

Nit: please find a way to reduce indentation (e.g. less if-nesting, or a 
utility function).

>                                                       } else {
>                                                               
> clear_scalar_id(dst_reg);
>                                                       }
> @@ -15961,6 +16021,32 @@ static void collect_linked_regs(struct 
> bpf_verifier_env *env,
>       }
>  }
>  
> +/*
> + * Set @reg to the sign-extension of the low 32 bits currently held by @src.
> + * A BPF_FLAG_SUBREG_SEXT-linked register came from a 32-bit sign
> + * extension (r0 = (s32)r0): it shares @src's low 32 bits and its high bits 
> are
> + * the sign-extension of that low field. Only the value fields are written;
> + * @reg's linkage fields (id, delta, flags) are left intact by
> + * the caller (___mark_reg_known touches only var_off/r64/r32). Callers must
> + * ensure no ADD_CONST delta is involved (see sync_linked_regs()).
> + */
> +static void reconstruct_sext32(struct bpf_reg_state *reg, struct 
> bpf_reg_state *src)

Nit: let's rename src -> known_reg, to make reading sync_linked_regs() simpler.

> +{
> +     s32 s32min = reg_s32_min(src);
> +     s32 s32max = reg_s32_max(src);
> +
> +     if (s32min == s32max) {
> +             /* Low 32 bits are constant -> the whole value is the sext 
> constant. */
> +             ___mark_reg_known(reg, (u64)(s64)s32min);
> +     } else {
> +             /* Sign-extension is monotonic over the signed-32 range. */
> +             reg_set_srange64(reg, (s64)s32min, (s64)s32max);
> +             reg_set_srange32(reg, s32min, s32max);
> +             reg->var_off = tnum_range((u64)(s64)s32min, (u64)(s64)s32max);

Note that known lower 32-bits of the known_reg->var_off are lost,
we might benefit from adding a dedicated tnum_sext().

> +             reg_bounds_sync(reg);
> +     }
> +}
> +
>  /* For all R in linked_regs, copy known_reg range into R
>   * if R->id == known_reg->id.
>   */
> @@ -15984,17 +16070,21 @@ static void sync_linked_regs(struct 
> bpf_verifier_env *env, struct bpf_verifier_s
>                * A low-32 linked register shares only the base's low 32 bits;
>                * the flag says how its high bits are derived. For
>                * BPF_FLAG_SUBREG_ZEXT they are zero (32-bit zero-extending 
> mov).
> +              * For BPF_FLAG_SUBREG_SEXT they are the sign-extension of the 
> low
> +              * field (32-bit sign extension).
>                * Rebuild it from known_reg's low 32 bits accordingly, but only
>                * when neither side carries an ADD_CONST delta -- with a delta
>                * the low bits differ from the base by that delta and the 
> combined
>                * subreg+ADD_CONST reconstruction isn't modeled here, so leave 
> reg
>                * unchanged (sound, just less precise).
>                */
> -             if (reg->flags & BPF_FLAG_SUBREG_ZEXT) {
> +             if (reg->flags & BPF_FLAG_SUBREG) {
>                       if (!((reg->flags | known_reg->flags) & 
> BPF_FLAG_ADD_CONST)) {
> -                             {
> +                             if (reg->flags & BPF_FLAG_SUBREG_SEXT) {
> +                                     reconstruct_sext32(reg, known_reg);
> +                             } else {

Let's move this branch to a dedicated utility function as well.

>                                       u32 saved_id = reg->id;
> -                                     u8 saved_subreg = reg->flags & 
> BPF_FLAG_SUBREG_ZEXT;
> +                                     u8 saved_subreg = reg->flags & 
> BPF_FLAG_SUBREG;
>  
>                                       /*
>                                        * reg = zext32(known_reg): its low 32 
> bits come from

...

Reply via email to