Re: [PATCH stable 6.6.y v3 1/4] bpf: Track equal scalars history on per-instruction level

[Zhenzhong Wu]

On second thought, I'll drop the cnt == 1 reset (and the matching cnt > 0)
and follow upstream's cnt > 1 in v4. It gives no benefit over upstream, it
only adds the corner cases the bot flagged. And The other two points the
bot raised (clearing id on overflow, collecting src/dst twice) are upstream
behaviour, so I will keep them as-is.Thanks for the pointer.



On Tue, Jun 16, 2026 at 1:51 PM Shung-Hsi Yu <shung-hsi.yu@xxxxxxxx> wrote:
>
> On Mon, Jun 15, 2026 at 12:58:38AM +0800, Zhenzhong Wu wrote:
> [...]
> > +/* For all R being scalar registers or spilled scalar registers
> > + * in verifier state, save R in linked_regs if R->id == id.
> > + * If there are too many Rs sharing same id, reset id for leftover Rs.
> > + */
> > +static void collect_linked_regs(struct bpf_verifier_state *vstate, u32 id,
> > +                             struct linked_regs *linked_regs)
> > +{
> > +     struct bpf_func_state *func;
> >       struct bpf_reg_state *reg;
> > +     int i, j;
> >
> > -     bpf_for_each_reg_in_vstate(vstate, state, reg, ({
> > -             if (reg->type == SCALAR_VALUE && reg->id == known_reg->id) {
> > +     for (i = vstate->curframe; i >= 0; i--) {
> > +             func = vstate->frame[i];
> > +             for (j = 0; j < BPF_REG_FP; j++) {
> > +                     reg = &func->regs[j];
> > +                     __collect_linked_regs(linked_regs, reg, id, i, j, true);
> > +             }
> > +             for (j = 0; j < func->allocated_stack / BPF_REG_SIZE; j++) {
> > +                     if (!is_spilled_reg(&func->stack[j]))
> > +                             continue;
> > +                     reg = &func->stack[j].spilled_ptr;
> > +                     __collect_linked_regs(linked_regs, reg, id, i, j, false);
> > +             }
> > +     }
> > +
> > +     if (linked_regs->cnt == 1)
> > +             linked_regs->cnt = 0;
>
> This part seems new, not found on the original commit, and also not in
> bpf-next. Can you add some more explaining (in the notes before your
> signed-off-by) regarding why this is needed?
>
> > +}
> [...]
> > @@ -14704,6 +14899,21 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env,
> >               return 0;
> >       }
> >
> > +     /* Push scalar registers sharing same ID to jump history,
> > +      * do this before creating 'other_branch', so that both
> > +      * 'this_branch' and 'other_branch' share this history
> > +      * if parent state is created.
> > +      */
> > +     if (BPF_SRC(insn->code) == BPF_X && src_reg->type == SCALAR_VALUE && src_reg->id)
> > +             collect_linked_regs(this_branch, src_reg->id, &linked_regs);
> > +     if (dst_reg->type == SCALAR_VALUE && dst_reg->id)
> > +             collect_linked_regs(this_branch, dst_reg->id, &linked_regs);
> > +     if (linked_regs.cnt > 0) {
>
> Same here, the original commit and bpf-next has the '> 1' conditional,
> where as your has '> 0'. Can you also added some explanation on this
> part?
>
> > +             err = push_jmp_history(env, this_branch, 0, linked_regs_pack(&linked_regs));
> > +             if (err)
> > +                     return err;
> > +     }
> > +
> ...