Re: [PATCH bpf-next 1/2] bpf: Forget ranges when refining tnum after JSET

[Paul Chaignon <paul.chaignon@gmail.com>]

On Wed, Jul 09, 2025 at 04:57:28PM -0700, Yonghong Song wrote:
> 
> 
> On 7/9/25 3:26 PM, Paul Chaignon wrote:
> > Syzbot reported a kernel warning due to a range invariant violation on
> > the following BPF program.
> > 
> >    0: call bpf_get_netns_cookie
> >    1: if r0 == 0 goto <exit>
> >    2: if r0 & Oxffffffff goto <exit>
> > 
> > The issue is on the path where we fall through both jumps.
> > 
> > That path is unreachable at runtime: after insn 1, we know r0 != 0, but
> > with the sign extension on the jset, we would only fallthrough insn 2
> > if r0 == 0. Unfortunately, is_branch_taken() isn't currently able to
> > figure this out, so the verifier walks all branches. The verifier then
> > refines the register bounds using the second condition and we end
> > up with inconsistent bounds on this unreachable path:
> > 
> >    1: if r0 == 0 goto <exit>
> >      r0: u64=[0x1, 0xffffffffffffffff] var_off=(0, 0xffffffffffffffff)
> >    2: if r0 & 0xffffffff goto <exit>
> >      r0 before reg_bounds_sync: u64=[0x1, 0xffffffffffffffff] var_off=(0, 0)
> >      r0 after reg_bounds_sync:  u64=[0x1, 0] var_off=(0, 0)
> > 
> > Improving the range refinement for JSET to cover all cases is tricky. We
> > also don't expect many users to rely on JSET given LLVM doesn't generate
> > those instructions. So instead of reducing false positives due to JSETs,
> > Eduard suggested we forget the ranges whenever we're narrowing tnums
> > after a JSET. This patch implements that approach.
> > 
> > Reported-by: syzbot+c711ce17dd78e5d4fdcf@syzkaller.appspotmail.com
> > Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > ---
> >   kernel/bpf/verifier.c | 4 ++++
> >   1 file changed, 4 insertions(+)
> > 
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 53007182b46b..e2fcea860755 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -16208,6 +16208,10 @@ static void regs_refine_cond_op(struct bpf_reg_state *reg1, struct bpf_reg_state
> >   		if (!is_reg_const(reg2, is_jmp32))
> >   			break;
> >   		val = reg_const_value(reg2, is_jmp32);
> > +		/* Forget the ranges before narrowing tnums, to avoid invariant
> > +		 * violations if we're on a dead branch.
> > +		 */
> > +		__mark_reg_unbounded(reg1);
> >   		if (is_jmp32) {
> >   			t = tnum_and(tnum_subreg(reg1->var_off), tnum_const(~val));
> >   			reg1->var_off = tnum_with_subreg(reg1->var_off, t);
> 
> The CI reports some invariant violation:
>   https://github.com/kernel-patches/bpf/actions/runs/16182458904/job/45681940946?pr=9283

AFAICS, these invariant violations predate this change. They seem to be
expected and caused by selftests crossing_64_bit_signed_boundary_2 and
crossing_32_bit_signed_boundary_2 which are both marked as "known
invariants violation". They look like fairly different violations as
they are not caused by JSET instructions.

I think it's still worth having the above change for JSET because we
lose only the ranges and not the tnums, whereas with an invariant
violation, we lose all info on the register. I'm looking into the two
other invariant violations to see if there's anything we can improve
there.

> 
> [ 283.030177] ------------[ cut here ]------------ [ 283.030517] verifier
> bug: REG INVARIANTS VIOLATION (false_reg2): range bounds violation
> u64=[0x8000000000000010, 0x800000000000000f] s64=[0x8000000000000010,
> 0x800000000000000f] u32=[0x10, 0xf] s32=[0x10, 0xf]
> var_off=(0x8000000000000000, 0x1f)(1)
> [ 283.032139] WARNING: CPU: 0 PID: 103 at kernel/bpf/verifier.c:2689
> reg_bounds_sanity_check+0x1dd/0x1f0 ... Probably this change triggered some
> other violations. Please take a look.
>