Re: [PATCH bpf-next 1/2] bpf: Skip bounds adjustment for conditional jumps on same register

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2025-10-23 at 19:26 +0800, KaFai Wan wrote:

[...]

> > @@ -16173,6 +16173,25 @@ static int is_pkt_ptr_branch_taken(struct
> > bpf_reg_state *dst_reg,
> >  static int is_branch_taken(struct bpf_reg_state *reg1, struct bpf_reg_state
> > *reg2,
> >                            u8 opcode, bool is_jmp32)
> >  {
> > +       if (reg1 == reg2) {
> > +               switch (opcode) {
> > +               case BPF_JGE:
> > +               case BPF_JLE:
> > +               case BPF_JSGE:
> > +               case BPF_JSLE:
> > +               case BPF_JEQ:
> > +               case BPF_JSET:
> 
> Others are fine, but BPF_JSET on the same register could be 0 (if value is 0).
> And it's unknown to take the branch if 0 within the range.

Right, missed that one.

> 
> > +                       return 1;
> > +               case BPF_JGT:
> > +               case BPF_JLT:
> > +               case BPF_JSGT:
> > +               case BPF_JSLT:
> > +               case BPF_JNE:
> > +                       return 0;
> > +               default:
> > +                       return -1;
> > +               }
> > +       }
> > 
> > But that's too much code for an artificial case.
> > Idk, either way is fine with me.
> 
> There is is_scalar_branch_taken() in is_branch_taken(), I missed it. I'll a)
> check the opcode one by one in is_scalar_branch_taken(), and b) keep this patch
> for unknown BPF_JSET branch.

Sounds good to me. Note that the logic is correct for both scalar and
non-scalar cases, so I don't think we have to constrain it to
is_scalar_branch_taken() (don't think there is a need to check if
pointer comparisons are allowed, as no new information is inferred
from comparisons with self).