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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f0a52150bc99aa4da1a25d6181975cd3c80a717f.camel@gmail.com>
Date: Thu, 23 Oct 2025 10:38:04 -0700
From: Eduard Zingerman <eddyz87@...il.com>
To: KaFai Wan <kafai.wan@...ux.dev>, Alexei Starovoitov
	 <alexei.starovoitov@...il.com>
Cc: Yonghong Song <yonghong.song@...ux.dev>, Alexei Starovoitov
 <ast@...nel.org>,  Daniel Borkmann <daniel@...earbox.net>, John Fastabend
 <john.fastabend@...il.com>, Andrii Nakryiko	 <andrii@...nel.org>, Martin
 KaFai Lau <martin.lau@...ux.dev>, Song Liu	 <song@...nel.org>, KP Singh
 <kpsingh@...nel.org>, Stanislav Fomichev	 <sdf@...ichev.me>, Hao Luo
 <haoluo@...gle.com>, Jiri Olsa <jolsa@...nel.org>,  Shuah Khan
 <shuah@...nel.org>, Paul Chaignon <paul.chaignon@...il.com>, Matan Shachnai
	 <m.shachnai@...il.com>, Luis Gerhorst <luis.gerhorst@....de>, 
	colin.i.king@...il.com, Harishankar Vishwanathan	
 <harishankar.vishwanathan@...il.com>, bpf <bpf@...r.kernel.org>, LKML	
 <linux-kernel@...r.kernel.org>, "open list:KERNEL SELFTEST FRAMEWORK"	
 <linux-kselftest@...r.kernel.org>, Kaiyan Mei <M202472210@...t.edu.cn>, 
 Yinhao Hu <dddddd@...t.edu.cn>
Subject: Re: [PATCH bpf-next 1/2] bpf: Skip bounds adjustment for
 conditional jumps on same register

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).