On 7/10/25 7:51 AM, Paul Chaignon wrote:
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@xxxxxxxxxxxxxxxxxxxxxxxxx
Suggested-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
Signed-off-by: Paul Chaignon <paul.chaignon@xxxxxxxxx>
---
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 double checked and that 'REG INVARIANTS VIOLATION' warning is indeed
not related to your patch so your change looks good to me. You can add
my ack like
Acked-by: Yonghong Song <yonghong.song@xxxxxxxxx>
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.
Thanks for looking at this!
[ 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.
