All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf 0/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution
@ 2026-10-27 20:14 Alexei Starovoitov
  2026-10-27 20:14 ` [PATCH bpf 1/2] " Alexei Starovoitov
  2026-10-27 20:14 ` [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop Alexei Starovoitov
  0 siblings, 2 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2026-10-27 20:14 UTC (permalink / raw)
  To: bpf; +Cc: daniel, andrii, eddyz87, memxor

From: Alexei Starovoitov <ast@kernel.org>

Sashiko found a bug in the review of patch 14 of [1]. It is in the tree
since scalar evolution widens packet pointers.

'range - umax' bytes from a packet pointer are in the packet. At the head
of a counted loop scalar evolution sets new bounds, and reg_bounds_sync()
makes umax smaller: 58 becomes 56 when the step is 8. The range does not
change. Then the verifier accepts 2 bytes that are not in the packet.

  prog                               bpf tree  [1]    this fix
  ---------------------------------  --------  -----  --------
  r6 += 8, access in the loop        loads     loads  rejected
  r6 -= 8, access in the loop        loads     loads  rejected
  access in the second iteration     loads     loads  rejected

These patches apply on top of [1].

veristat shows the same verdicts and the same number of insns for all progs
of 36 weekly builds of Cilium (5032 progs), of sched-ext/scx (563 progs)
and of the selftests. test_progs, test_progs-no_alu32, test_progs-cpuv4,
test_progs-bpf_gcc and test_verifier have no new failure on x86-64.

[1] https://lore.kernel.org/bpf/20261027190314.1440199-1-alexei.starovoitov@gmail.com/

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov (2):
  bpf: Keep the valid bytes of a packet pointer in scalar evolution
  selftests/bpf: Test the valid bytes of a packet pointer in a counted
    loop

 kernel/bpf/scev.c                             |   3 +
 kernel/bpf/verifier.c                         |  18 +-
 .../selftests/bpf/progs/verifier_scev.c       | 158 ++++++++++++++++++
 3 files changed, 177 insertions(+), 2 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH bpf 1/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution
  2026-10-27 20:14 [PATCH bpf 0/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution Alexei Starovoitov
@ 2026-10-27 20:14 ` Alexei Starovoitov
  2026-10-27 20:59   ` bot+bpf-ci
  2026-10-27 21:04   ` sashiko-bot
  2026-10-27 20:14 ` [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop Alexei Starovoitov
  1 sibling, 2 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2026-10-27 20:14 UTC (permalink / raw)
  To: bpf; +Cc: daniel, andrii, eddyz87, memxor

From: Alexei Starovoitov <ast@kernel.org>

check_packet_access() accepts an access when umax + off + size <= range.
Thus 'range - umax' bytes from a packet pointer are in the packet, and
nothing can make umax smaller without a change of the range. For this
reason commit b88822d2584f ("bpf: Fix packet range of pointers sharing an
id") removed reg_bounds_sync() from the arithmetic on packet pointers.

At the head of a counted loop scalar evolution gives new bounds to a packet
pointer, in bpf_set_reg_range() and in bpf_reg_union(). The two functions
call reg_bounds_sync(). Then the step or var_off makes umax smaller.

     r4 &= 0x38
     if r4 > 50 goto 2f         ; r4 is 0, 8 .. 48, but umax is 50
     r6 = pkt
     r6 += r4
     r5 = r6
     r5 += 8
     if r5 > pkt_end goto 2f    ; 8 bytes from r6 are in the packet
  1: r0 = *(u16 *)(r6 + 0)
     r6 += 8
     r9 += 1
     if r9 < 2 goto 1b

  r6                       umax  range  valid bytes
  -----------------------  ----  -----  ------------
  in front of the loop       50     58  8
  in all iterations          58     58  0
  after the step of 8        56     58  2, incorrect

In the second iteration r6 can be pkt_end. The verifier accepts the prog,
and the prog reads 2 bytes after the packet. A kernel without scalar
evolution rejects it.

Make the range smaller by the same number as umax.

The range can also be a mark: the pointer is at pkt_end or beyond pkt_end.
The mark is valid in the first iteration only. Remove it when the pointer
is widened. This does not change a verdict. The arithmetic in the loop
removes the mark too, so the state that returns to the head was not pruned,
and the verifier made one more pass with the two branches of each compare.

Fixes: 6ccf49f79327 ("bpf: Use SCEV to widen bounded loops")
Closes: https://lore.kernel.org/bpf/sashiko-outbox-187068@kernel.org/
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 kernel/bpf/scev.c     |  3 +++
 kernel/bpf/verifier.c | 18 ++++++++++++++++--
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c
index 7b82fd79f9cc..abc4ba5ec1a9 100644
--- a/kernel/bpf/scev.c
+++ b/kernel/bpf/scev.c
@@ -2837,6 +2837,9 @@ int bpf_widen_scev_regs(struct bpf_verifier_env *env, struct bpf_verifier_state
 				bpf_log(log, "\n");
 			}
 			scratch_widened_reg_id(env, reg);
+			/* "at pkt_end" and "beyond pkt_end" are not valid in all iterations */
+			if (type_is_pkt_pointer(reg->type))
+				reg->range = max(reg->range, 0);
 			err = bpf_set_reg_range(env, reg, bounds.r64, bounds.r32,
 						bounds.base, bounds.step);
 			if (err)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 02b2915f0014..cf7b64de1a4d 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -17739,6 +17739,20 @@ bool bpf_same_memory_origin(const struct bpf_reg_state *reg_a,
 	}
 }
 
+/*
+ * 'range - umax' bytes from a packet pointer are in the packet. reg_bounds_sync() can make umax
+ * smaller: with a step of 8, or with 3 zero bits in var_off, 58 becomes 56. Make the range
+ * smaller by the same number, so that the number of bytes does not grow.
+ */
+static void sync_widened_reg(struct bpf_reg_state *reg)
+{
+	u64 umax = reg_umax(reg);
+
+	reg_bounds_sync(reg);
+	if (type_is_pkt_pointer(reg->type) && reg->range > 0)
+		reg->range = max_t(s64, reg->range - (s64)(umax - reg_umax(reg)), 0);
+}
+
 int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 		      struct cnum64 r64, struct cnum32 r32, u16 base, u16 step)
 {
@@ -17747,7 +17761,7 @@ int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 	reg->step = step;
 	reg->base = base;
 	reg->var_off = tnum_unknown;
-	reg_bounds_sync(reg); /* this should infer the tnum alignment */
+	sync_widened_reg(reg); /* this should infer the tnum alignment */
 	return reg_bounds_sanity_check(env, reg, "bpf_set_reg_range");
 }
 
@@ -17792,7 +17806,7 @@ int bpf_reg_union(struct bpf_verifier_env *env, struct bpf_reg_state *acc,
 		acc->base = base;
 		acc->step = step;
 	}
-	reg_bounds_sync(acc);
+	sync_widened_reg(acc);
 	return reg_bounds_sanity_check(env, acc, "bpf_reg_union");
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop
  2026-10-27 20:14 [PATCH bpf 0/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution Alexei Starovoitov
  2026-10-27 20:14 ` [PATCH bpf 1/2] " Alexei Starovoitov
@ 2026-10-27 20:14 ` Alexei Starovoitov
  2026-10-27 20:59   ` bot+bpf-ci
  2026-10-27 20:59   ` sashiko-bot
  1 sibling, 2 replies; 7+ messages in thread
From: Alexei Starovoitov @ 2026-10-27 20:14 UTC (permalink / raw)
  To: bpf; +Cc: daniel, andrii, eddyz87, memxor

From: Alexei Starovoitov <ast@kernel.org>

In each test r6 = pkt + r4. r4 is 0, 8 .. 48, but its umax is 50.

  test                                      umax gets smaller    result
  ----------------------------------------  -------------------  ------
  widen_pkt_pointer_step_lowers_umax        at the loop entry    reject
  widen_pkt_pointer_step_lowers_umax_down   the same, r6 -= 8    reject
  clamp_pkt_pointer_step_lowers_umax        at the back edge     reject
  union_pkt_pointer_var_off_lowers_umax     r6 is r6 or r6 + 8   reject
  widen_pkt_pointer_keeps_valid_bytes       at the loop entry    loads

The first three progs load without the fix. The verifier rejects the fourth
prog without the fix too, but only in the second pass. The test looks at
the range in the message. The last prog reads only bytes that the compare
with pkt_end covers.

widen_pkt_pointer_beyond_pkt_end makes sure that the mark "beyond pkt_end"
is not in the state at the head of the loop.

Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 .../selftests/bpf/progs/verifier_scev.c       | 158 ++++++++++++++++++
 1 file changed, 158 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/verifier_scev.c b/tools/testing/selftests/bpf/progs/verifier_scev.c
index fa01a616af9d..7b399bae0c60 100644
--- a/tools/testing/selftests/bpf/progs/verifier_scev.c
+++ b/tools/testing/selftests/bpf/progs/verifier_scev.c
@@ -3936,4 +3936,162 @@ __naked void clamp_pkt_pointer_keeps_range_of_entry(void)
 	: __clobber_all);
 }
 
+/*
+ * r4 is 0, 8 .. 48, but its umax is 50. @bytes from r6 are in the packet: the range of r6 is
+ * 50 + @bytes.
+ */
+#define PKT_PLUS_R4(start, bytes)					\
+	"r7 = *(u32 *)(r1 + %[data_end]);"				\
+	"r6 = *(u32 *)(r1 + %[data]);"					\
+	"r4 = *(u32 *)(r1 + %[mark]);"					\
+	"r4 &= 0x38;"							\
+	"if r4 > 50 goto 9f;"						\
+	"r6 += " #start ";"						\
+	"r6 += r4;"							\
+	"r5 = r6;"							\
+	"r5 += " #bytes ";"						\
+	"if r5 > r7 goto 9f;"
+
+#define PKT_IMMS							\
+	__imm_const(data, offsetof(struct __sk_buff, data)),		\
+	__imm_const(data_end, offsetof(struct __sk_buff, data_end)),	\
+	__imm_const(mark, offsetof(struct __sk_buff, mark))
+
+/*
+ * r6 is 0..58 at the head, and 0..56 with the step of 8. The range must go from 58 to 56 too:
+ * in the second iteration no byte from r6 is in the packet.
+ */
+SEC("tc")
+__failure __msg("invalid access to packet, off=56 size=2, R6(id=2,off=56,r=56)")
+__naked void widen_pkt_pointer_step_lowers_umax(void)
+{
+	asm volatile (
+	PKT_PLUS_R4(0, 8)
+	"r9 = 0;"
+"1:"
+	"r0 = *(u16 *)(r6 + 0);"
+	"r6 += 8;"
+	"r9 += 1;"
+	"if r9 < 2 goto 1b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
+/* The pointer goes down. 8 bytes from the first r6 are in the packet, not 10. */
+SEC("tc")
+__failure __msg("invalid access to packet, off=120 size=2, R6(id=2,off=120,r=120)")
+__naked void widen_pkt_pointer_step_lowers_umax_down(void)
+{
+	asm volatile (
+	PKT_PLUS_R4(64, 8)
+	"r9 = 0;"
+"1:"
+	"r0 = *(u16 *)(r6 + 8);"
+	"r6 += -8;"
+	"r9 += 1;"
+	"if r9 < 2 goto 1b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
+/* The access is in the second iteration only: the verifier gets to it after the clamp */
+SEC("tc")
+__failure __msg("invalid access to packet, off=56 size=2, R6(id=3,off=56,r=56)")
+__naked void clamp_pkt_pointer_step_lowers_umax(void)
+{
+	asm volatile (
+	PKT_PLUS_R4(0, 8)
+	"r9 = 0;"
+	"w1 = 0;"
+	"*(u32 *)(r10 - 8) = r1;"
+"1:"
+	"w1 = *(u32 *)(r10 - 8);"
+	"if w1 == 0 goto 2f;"
+	"r0 = *(u16 *)(r6 + 0);"
+"2:"
+	"w1 = 1;"
+	"*(u32 *)(r10 - 8) = r1;"
+	"r6 += 8;"
+	"r9 += 1;"
+	"if r9 < 2 goto 1b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
+/* r6 is r6 or r5 = r6 + 8 at the head. Here var_off lowers umax. */
+SEC("tc")
+__failure __msg("invalid access to packet, off=56 size=2, R6(id=2,off=56,r=56)")
+__naked void union_pkt_pointer_var_off_lowers_umax(void)
+{
+	asm volatile (
+	PKT_PLUS_R4(0, 8)
+	"r9 = 0;"
+"1:"
+	"r0 = *(u16 *)(r6 + 0);"
+	"r6 = r5;"
+	"r9 += 1;"
+	"if r9 < 2 goto 1b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
+/* 16 bytes from r6 are in the packet: two iterations read 8 bytes each */
+SEC("tc")
+__success __retval(0)
+__naked void widen_pkt_pointer_keeps_valid_bytes(void)
+{
+	asm volatile (
+	PKT_PLUS_R4(0, 16)
+	"r9 = 0;"
+"1:"
+	"r0 = *(u64 *)(r6 + 0);"
+	"r6 += 8;"
+	"r9 += 1;"
+	"if r9 < 2 goto 1b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
+/*
+ * r6 is beyond pkt_end in front of the loop and comes back into the packet. The mark is not for
+ * all iterations: the verifier must look at the two branches of the compare in the first pass.
+ */
+SEC("tc")
+__success
+__log_level(2)
+__msg("widening r6 to 32..96 step 32")
+__msg("6: R6=pkt(id=1,r=0,smin=umin=smin32=umin32=32,smax=umax=smax32=umax32=96")
+__naked void widen_pkt_pointer_beyond_pkt_end(void)
+{
+	asm volatile (
+	"r7 = *(u32 *)(r1 + %[data_end]);"
+	"r6 = *(u32 *)(r1 + %[data]);"
+	"r6 += 96;"
+	"if r6 > r7 goto 1f;"
+	"goto 9f;"
+"1:"
+	"r9 = 0;"
+"2:"
+	"if r6 > r7 goto 3f;"
+	"r0 = *(u8 *)(r6 - 1);"
+"3:"
+	"r6 += -32;"
+	"r9 += 1;"
+	"if r9 < 3 goto 2b;"
+"9:"
+	"r0 = 0;"
+	"exit;"
+	:: PKT_IMMS : __clobber_all);
+}
+
 char _license[] SEC("license") = "GPL";
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop
  2026-10-27 20:14 ` [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop Alexei Starovoitov
@ 2026-10-27 20:59   ` bot+bpf-ci
  2026-10-27 20:59   ` sashiko-bot
  1 sibling, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-10-27 20:59 UTC (permalink / raw)
  To: alexei.starovoitov, bpf
  Cc: daniel, andrii, eddyz87, memxor, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 4146 bytes --]

> selftests/bpf: Test the valid bytes of a packet pointer in a counted loop

TL;DR:
- Selftest clamp_pkt_pointer_step_lowers_umax may expect wrong __msg: r=56 vs
  r=54 (unrun, from reading code)
- If so, the clamp may subtract rounding twice and wrongly reject a valid
  packet load

> diff --git a/tools/testing/selftests/bpf/progs/verifier_scev.c b/tools/testing/selftests/bpf/progs/verifier_scev.c
> --- a/tools/testing/selftests/bpf/progs/verifier_scev.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_scev.c

[ ... ]

> +/* The access is in the second iteration only: the verifier gets to it after the clamp */
> +SEC("tc")
> +__failure __msg("invalid access to packet, off=56 size=2, R6(id=3,off=56,r=56)")
> +__naked void clamp_pkt_pointer_step_lowers_umax(void)
> +{
> +	asm volatile (
> +	PKT_PLUS_R4(0, 8)
> +	"r9 = 0;"
> +	"w1 = 0;"
> +	"*(u32 *)(r10 - 8) = r1;"
> +"1:"
> +	"w1 = *(u32 *)(r10 - 8);"
> +	"if w1 == 0 goto 2f;"
> +	"r0 = *(u16 *)(r6 + 0);"
> +"2:"
> +	"w1 = 1;"
> +	"*(u32 *)(r10 - 8) = r1;"
> +	"r6 += 8;"
> +	"r9 += 1;"
> +	"if r9 < 2 goto 1b;"
> +"9:"
> +	"r0 = 0;"
> +	"exit;"
> +	:: PKT_IMMS : __clobber_all);
> +}

[ ... ]

Will this __msg() match what the verifier prints once the previous patch in
the series is applied?  The expected "R6(id=3,off=56,r=56)" assumes r6 still
has r=58 when the clamp runs at the back edge.

Tracing the prog (r4 has step 1 because the BPF_AND calls reg_step_reset(),
so umax stays 50 after 'if r4 > 50'):

1. Before the loop r6 = pkt + r4 is [0,50] with var_off (0;0x38) and id=1.
   'if r5 > r7' gives it r=58.

2. On first arrival at the loop header handle_loop_entry_exit() calls
   bpf_widen_scev_regs().  linear_bounds() gives 0..58 step 8 and
   bpf_set_reg_range() reaches sync_widened_reg():

    u64 umax = reg_umax(reg);          /* 58 */
    reg_bounds_sync(reg);              /* umax 56 via step 8 */
    if (type_is_pkt_pointer(reg->type) && reg->range > 0)
        reg->range = max_t(s64, reg->range - (s64)(umax - reg_umax(reg)), 0);

   so r goes from 58 to 56.  This is the same state in which
   widen_pkt_pointer_step_lowers_umax expects "R6(id=2,off=56,r=56)".

3. In the first pass fp-8 holds 0, so 'if w1 == 0 goto 2f' skips the load.
   'r6 += 8' is a known add, so adjust_ptr_min_max_vals() keeps the range:
   r6 is [8,64] with r=56.

4. At the back edge maybe_clamp_scev_regs() calls bpf_clamp_scev_regs().  It
   takes linear_bounds() from the entry checkpoint (range 58, giving 0..58
   step 8) and gives the reg id=3, then calls bpf_set_reg_range() on the
   back-edge reg, whose range is already 56.  sync_widened_reg() subtracts
   the same 2 bytes again: umax 58 -> 56 and range 56 -> 54.

5. regsafe() does not prune against the header checkpoint (rold->range is 56
   and rcur->range is 54, and fp-8 is 0 vs 1).  The second pass reaches the
   load, and check_mem_region_access() -> __check_mem_access() would report:

    invalid access to packet, off=56 size=2, R6(id=3,off=56,r=54)

Should the test expect r=54 here?

If r=56 is the intended value, then the clamp path in the previous patch
looks too strict instead.  bpf_clamp_scev_regs() takes the bounds from the
entry reg but keeps the range of the back-edge reg, which already lost the
header rounding.

A prog that this would wrongly reject is clamp_pkt_pointer_step_lowers_umax
with PKT_PLUS_R4(0, 16) and an 8-byte load in the second iteration only.  In
the second iteration r6 = r6_entry + 8 and r6_entry + 16 <= pkt_end, so the
load is in bounds.  The clamped r6 has umax 56 and r=62, so 56 + 8 > 62 and
the load is rejected.  A verifier without scalar evolution accepts it (second
iteration r6 umax 58, r=66), and so did the scalar evolution code before the
previous patch.  Should the clamp start from the entry reg's range so that
the rounding is subtracted only once?

This was derived by reading the code, not by running the test.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40578692597

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf 1/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution
  2026-10-27 20:14 ` [PATCH bpf 1/2] " Alexei Starovoitov
@ 2026-10-27 20:59   ` bot+bpf-ci
  2026-10-27 21:04   ` sashiko-bot
  1 sibling, 0 replies; 7+ messages in thread
From: bot+bpf-ci @ 2026-10-27 20:59 UTC (permalink / raw)
  To: alexei.starovoitov, bpf
  Cc: daniel, andrii, eddyz87, memxor, ast, andrii, daniel, martin.lau,
	eddyz87, yonghong.song, mason, ihor.solodrai

[-- Attachment #1: Type: text/plain, Size: 8566 bytes --]

> bpf: Keep the valid bytes of a packet pointer in scalar evolution

TL;DR:
- Back-edge clamp may overstate packet range, so verifier could accept an
  out-of-bounds packet read (not new)
- Range may be lowered twice on clamp, possibly rejecting safe programs
- bpf_reg_union() may hit "union of incompatible registers" verifier bug on
  valid programs

> diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c
> --- a/kernel/bpf/scev.c
> +++ b/kernel/bpf/scev.c

[ ... ]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17768,6 +17768,20 @@ bool bpf_same_memory_origin(const struct bpf_reg_state *reg_a,
>  	}
>  }
>
> +/*
> + * 'range - umax' bytes from a packet pointer are in the packet. reg_bounds_sync() can make umax
> + * smaller: with a step of 8, or with 3 zero bits in var_off, 58 becomes 56. Make the range
> + * smaller by the same number, so that the number of bytes does not grow.
> + */
> +static void sync_widened_reg(struct bpf_reg_state *reg)
> +{
> +	u64 umax = reg_umax(reg);
> +
> +	reg_bounds_sync(reg);
> +	if (type_is_pkt_pointer(reg->type) && reg->range > 0)
> +		reg->range = max_t(s64, reg->range - (s64)(umax - reg_umax(reg)), 0);
> +}
> +

Does this cover the back-edge clamp path?  The commit message says "Make
the range smaller by the same number as umax", but when this is reached
through bpf_clamp_scev_regs()->bpf_set_reg_range(), the umax captured here is
read after bpf_set_reg_range() has already overwritten r64.

In bpf_clamp_scev_regs() umax drops twice:

  1. from the register's post-body umax to the widened bound, when
     bpf_set_reg_range() overwrites r64
  2. by the delta from reg_bounds_sync()

Only the second drop is taken off reg->range.  That is harmless when the
loop body only adds constants, because the range then matches the widened
state (but see the double lowering below).  It is not harmless when the body
raises the range with a new compare against pkt_end, because that range
refers to the post-body umax.

Take the clamp_pkt_pointer_step_lowers_umax shape (access only after the
clamp) with a re-check added to the loop body.  PKT_PLUS_R4(0, 8): r6 is
[0,50] with range 58, and the real value is x0 <= 48 with
pkt_len >= x0 + 8:

  - widening: r6 becomes [0,56], range 56
  - body: r6 += 8 gives [8,64]; then r5 = r6; r5 += 8;
    if r5 > pkt_end goto out.  find_good_pkt_pointers() gives
    r6.range = max(56, 72) = 72
  - clamp: [0,58] becomes [0,56], and range becomes 72 - 2 = 70, so the
    verifier counts 14 valid bytes

Really r6 = x0 + 8 and pkt_len can be x0 + 16, so only 8 bytes are valid.  A
*(u32 *)(r6 + 10) in the second iteration passes 56 + 10 + 4 <= 70 and reads
up to 6 bytes past the packet.  Before the patch the range was 72, or 16
valid bytes, so this problem is not new, but the change does not cover it.  A
kernel without scalar evolution rejects the access: 58 + 10 + 4 > 66.

Should the delta be measured from the register's umax before
bpf_set_reg_range() overwrites r64, or should the range be rebuilt from the
widened state?

>  int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>  		      struct cnum64 r64, struct cnum32 r32, u16 base, u16 step)
>  {
> @@ -17776,7 +17790,7 @@ int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>  	reg->step = step;
>  	reg->base = base;
>  	reg->var_off = tnum_unknown;
> -	reg_bounds_sync(reg); /* this should infer the tnum alignment */
> +	sync_widened_reg(reg); /* this should infer the tnum alignment */
>  	return reg_bounds_sanity_check(env, reg, "bpf_set_reg_range");
>  }

bpf_clamp_scev_regs() also calls bpf_set_reg_range().  Doesn't that lower
the range a second time by the same delta on every back edge?

In the widening path the range comes from the loop entry and goes with the
entry bounds, so lowering it by (58 - 56) is correct.  In the clamp path,
reg is the register at the back edge.  Its range is the widened range,
already lowered and carried through the body by constant adds, which keep
range.  bpf_clamp_scev_regs() computes linear_bounds(entry_reg, ...) from the
same loop-entry register, so it installs the same pre-sync r64, and
sync_widened_reg() then subtracts the same delta again.

Take the selftest widen_pkt_pointer_keeps_valid_bytes from the follow-up
commit 7c4d9d4f7f26, PKT_PLUS_R4(0, 16): r6 is [0,50] with range 66.

  - widening: r64 [0,58] is synced to [0,56] and range becomes 66 - 2 = 64.
    Checkpoint W is created, and the u64 load passes (56 + 8 <= 64)
  - body: r6 += 8 gives [8,64], range 64
  - back edge, clamp: r64 [0,58] is synced to [0,56] and range becomes
    64 - 2 = 62
  - bpf_is_state_visited(): regsafe() for PTR_TO_PACKET has

        else if (rold->range > rcur->range)
                return false;

    and 64 > 62, so there is no hit
  - the clamped state runs the body again: 56 + 8 > 62, giving "invalid
    access to packet, off=56 size=8, R6(id=3,off=56,r=62)"

That program is safe.  It loads before this patch, where the widened and
clamped ranges are both 66 and the state is pruned, and it loads on a kernel
without scalar evolution.  The follow-up commit 7c4d9d4f7f26 expects
__success for it.

By the same steps, clamp_pkt_pointer_step_lowers_umax gives r=54, not the
r=56 its __msg expects.

Can the range be lowered only once for a given widened register, so that the
clamped state matches the widened checkpoint?

[ ... ]

> @@ -17821,7 +17835,7 @@ int bpf_reg_union(struct bpf_verifier_env *env, struct bpf_reg_state *acc,
>  		acc->base = base;
>  		acc->step = step;
>  	}
> -	reg_bounds_sync(acc);
> +	sync_widened_reg(acc);
>  	return reg_bounds_sanity_check(env, acc, "bpf_reg_union");
>  }

Can this make a valid program fail with "union of incompatible registers"?

sync_widened_reg() now changes acc->range inside the union loop.
union_any_reg() calls bpf_reg_union(env, acc, leaf_reg) once per leaf of the
union expression, and its comment says "bpf_compute_loop_iters() checked that
every leaf can be unioned into acc".  That check is
is_widenable_any()->bpf_reg_union_compatible().  It runs once, on the
loop-entry registers, and it does:

    return !memcmp(a, b, offsetof(struct bpf_reg_state, var_off)) && ...

The memcmp() covers the union that holds range.  Before this patch
bpf_reg_union() never wrote range, so the check stayed true for the whole
loop.  Now sync_widened_reg(acc) lowers acc->range whenever
reg_bounds_sync() lowers umax, while the leaves that follow still carry the
loop-entry range.  bpf_reg_union_compatible(acc, leaf) then fails, and
bpf_reg_union() does verifier_bug(env, "union of incompatible registers")
and returns -EFAULT.  With CONFIG_DEBUG_KERNEL, verifier_bug() also fires
WARN_ONCE.

mk_any() puts the register itself first, so the leaves are visited in the
order (r, latch leaves...).  The failure is hit when the bounds narrow on any
union that is not the last one.  One way to get there is a packet pointer
whose bounds were never synced.  adjust_ptr_min_max_vals() skips
reg_bounds_sync() for a known addend (see b88822d2584f), so a constant add
after a variable add leaves them unsynced:

    r4 &= 0x38
    if r4 > 50 goto 9f
    r6 = pkt
    r6 += r4                  ; [0,50] var_off (0;0x38), synced
    r6 += 8                   ; [8,58] var_off (0;0x78), not synced
    r5 = r6
    r5 += 8
    if r5 > pkt_end goto 9f   ; r5.range = r6.range = 66
    r9 = 0
 1: if r6 > pkt_end goto 9f
    r6 = r5                   ; header SCEV of r6: ANY(r6, r5)
    r9 += 1
    if r9 < 2 goto 1b
 9: r0 = 0; exit

The first union is acc with itself.  reg_bounds_sync() turns var_off
(0;0x78) into (0;0x38), so umax goes from 58 to 56 and acc->range from 66 to
64.  The second leaf r5 still has range 66, so bpf_reg_union_compatible() is
false and the load fails with "verifier bug: union of incompatible
registers".  Before the patch this program loads.

A phi with three leaves fails the same way when the second union narrows, as
it does in the follow-up test union_pkt_pointer_var_off_lowers_umax with one
more leaf.

Would it work to adjust the range once, in union_any_reg() after the last
leaf, rather than in each bpf_reg_union() call?  Alternatively, could range
be left out of the compatibility test inside the loop?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/40578692597

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop
  2026-10-27 20:14 ` [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop Alexei Starovoitov
  2026-10-27 20:59   ` bot+bpf-ci
@ 2026-10-27 20:59   ` sashiko-bot
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-27 20:59 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf: missing reg->range reset in bpf_clamp_scev_regs() on loop backedges
- [High] bpf: incorrect packet range in sync_widened_reg() via reg_umax() on smin < 0
--

commit 1beef654c7066a8a9756b909f171d71afd35955c
Author: Alexei Starovoitov <ast@kernel.org>

selftests/bpf: Test the valid bytes of a packet pointer in a counted loop

This commit adds BPF verifier selftests covering packet pointer range
adjustments when scalar evolution widens or clamps registers in counted
loops.

> diff --git a/tools/testing/selftests/bpf/progs/verifier_scev.c b/tools/testing/selftests/bpf/progs/verifier_scev.c
> index 37434efd14909..adaf926b80553 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_scev.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_scev.c
> @@ -3737,4 +3737,162 @@ __naked void post_cond_jlt32_copy(void)

[ ... ]

> +/* The pointer goes down. 8 bytes from the first r6 are in the packet, not 10. */
> +SEC("tc")
> +__failure __msg("invalid access to packet, off=120 size=2, R6(id=2,off=120,r=120)")
> +__naked void widen_pkt_pointer_step_lowers_umax_down(void)
> +{
> +	asm volatile (
> +	PKT_PLUS_R4(64, 8)
> +	"r9 = 0;"
> +"1:"
> +	"r0 = *(u16 *)(r6 + 8);"
> +	"r6 += -8;"
> +	"r9 += 1;"
> +	"if r9 < 2 goto 1b;"
> +"9:"
> +	"r0 = 0;"
> +	"exit;"
> +	:: PKT_IMMS : __clobber_all);
> +}

[Severity: High]
In widen_pkt_pointer_step_lowers_umax_down(), starting at offset 64 keeps
bounds.r64 non-negative ([56, 114]), but what happens in
sync_widened_reg() when a downward-stepping packet pointer produces a
signed range with smin < 0 <= smax?

In kernel/bpf/scev.c:linear_bounds(), a negative slope subtracts from
min_val in signed arithmetic and constructs cnum64_from_srange(min_val,
max_val). Whenever min_val < 0 <= max_val, cnum64_urange_overflow() is
true and reg_umax(reg) in kernel/bpf/cnum_defs.h:cnum64_umax() returns
U64_MAX instead of max_val:

kernel/bpf/verifier.c:sync_widened_reg() {
	u64 umax = reg_umax(reg);

	reg_bounds_sync(reg);
	if (type_is_pkt_pointer(reg->type) && reg->range > 0)
		reg->range = max_t(s64, reg->range - (s64)(umax - reg_umax(reg)), 0);
}

This causes two issues depending on whether reg_bounds_sync() eliminates
the negative portion of the range:

1. If smin < 0 before reg_bounds_sync() and smin >= 0 after
reg_bounds_sync() (for example, r4 in [6, 50] with var_off = (0x0; 0x38),
r6 = pkt + r4 with range = 58, stepping by -8 for 2 iterations gives
bounds.r64 = [-2, 50] with step = 8, base = 0, which
cnum64_intersect_linear() narrows to [0, 48]), then umax before
reg_bounds_sync() is U64_MAX and reg_umax(reg) after is 48. The delta
(s64)(U64_MAX - 48) is -49, so reg->range is inflated from 58 to 107 while
reg_umax(r6) is 48, allowing out-of-bounds packet access past pkt_end.

2. If smin < 0 both before and after reg_bounds_sync() (for example,
PKT_PLUS_R4(0, 8) stepping by -8 for 2 iterations gives bounds.r64 =
[-8, 50] with step = 8, which reg_bounds_sync() narrows to [-8, 48]),
reg_umax(reg) is U64_MAX both before and after reg_bounds_sync(). Then
umax - reg_umax(reg) is 0 and reg->range stays at 58 even though
reg_smax(reg) dropped from 50 to 48, allowing r5 = r6; r5 += 8 (yielding
r64 = [0, 56], range = 58) to read 2 bytes past pkt_end.

Should sync_widened_reg() track the upper bound delta using reg_smax(reg)
instead of reg_umax(reg)?

> +
> +/* The access is in the second iteration only: the verifier gets to it after the clamp */
> +SEC("tc")
> +__failure __msg("invalid access to packet, off=56 size=2, R6(id=3,off=56,r=56)")
> +__naked void clamp_pkt_pointer_step_lowers_umax(void)
> +{
> +	asm volatile (
> +	PKT_PLUS_R4(0, 8)
> +	"r9 = 0;"
> +	"w1 = 0;"
> +	"*(u32 *)(r10 - 8) = r1;"
> +"1:"
> +	"w1 = *(u32 *)(r10 - 8);"
> +	"if w1 == 0 goto 2f;"
> +	"r0 = *(u16 *)(r6 + 0);"
> +"2:"
> +	"w1 = 1;"
> +	"*(u32 *)(r10 - 8) = r1;"
> +	"r6 += 8;"
> +	"r9 += 1;"
> +	"if r9 < 2 goto 1b;"
> +"9:"
> +	"r0 = 0;"
> +	"exit;"
> +	:: PKT_IMMS : __clobber_all);
> +}
> +
> +/* r6 is r6 or r5 = r6 + 8 at the head. Here var_off lowers umax. */
> +SEC("tc")
> +__failure __msg("invalid access to packet, off=56 size=2, R6(id=2,off=56,r=56)")
> +__naked void union_pkt_pointer_var_off_lowers_umax(void)
> +{
> +	asm volatile (
> +	PKT_PLUS_R4(0, 8)
> +	"r9 = 0;"
> +"1:"
> +	"r0 = *(u16 *)(r6 + 0);"
> +	"r6 = r5;"
> +	"r9 += 1;"
> +	"if r9 < 2 goto 1b;"
> +"9:"
> +	"r0 = 0;"
> +	"exit;"
> +	:: PKT_IMMS : __clobber_all);
> +}
> +
> +/* 16 bytes from r6 are in the packet: two iterations read 8 bytes each */
> +SEC("tc")
> +__success __retval(0)
> +__naked void widen_pkt_pointer_keeps_valid_bytes(void)
> +{
> +	asm volatile (
> +	PKT_PLUS_R4(0, 16)
> +	"r9 = 0;"
> +"1:"
> +	"r0 = *(u64 *)(r6 + 0);"
> +	"r6 += 8;"
> +	"r9 += 1;"
> +	"if r9 < 2 goto 1b;"
> +"9:"
> +	"r0 = 0;"
> +	"exit;"
> +	:: PKT_IMMS : __clobber_all);
> +}

[Severity: High]
Do clamp_pkt_pointer_step_lowers_umax() and
widen_pkt_pointer_keeps_valid_bytes() pass as written when
bpf_clamp_scev_regs() runs at the loop backedge?

In handle_loop_entry_exit(), bpf_split_cur_state() snapshots the
pre-widened state into entry->entry_state before calling
bpf_widen_scev_regs(), so entry_reg retains the unwidened r64 = [0, 50]
and range (58 in clamp_pkt_pointer_step_lowers_umax, 66 in
widen_pkt_pointer_keeps_valid_bytes).

At loop entry, bpf_widen_scev_regs() calls bpf_set_reg_range() ->
sync_widened_reg(), which lowers umax from 58 to 56 and subtracts 2 from
reg->range (producing 56 and 64, respectively).

At the backedge after the first iteration, handle_loop_entry_exit() ->
maybe_clamp_scev_regs() -> bpf_clamp_scev_regs() recomputes
linear_bounds() from the unwidened entry_reg (again giving bounds.r64 =
[0, 58], step = 8) and calls bpf_set_reg_range() on cur_func_state's reg
without resetting reg->range to max(entry_reg->range, 0):

kernel/bpf/scev.c:bpf_clamp_scev_regs() {
	...
	linear_bounds(entry_reg, max_header_count, &coefs, &bounds);
	...
	scratch_widened_reg_id(env, reg);
	err = bpf_set_reg_range(env, reg, bounds.r64, bounds.r32,
				bounds.base, bounds.step);
}

Inside bpf_set_reg_range(), reg->r64 is set to [0, 58] while reg->range is
still 56 (or 64) from iteration 1, so sync_widened_reg() sees umax drop
from 58 to 56 a second time and subtracts another 2 from reg->range:

1. In clamp_pkt_pointer_step_lowers_umax(), reg->range becomes 54 at the
backedge instead of 56, so the second iteration fails with R6(id=3,off=56,
r=54), mismatching the expected R6(id=3,off=56,r=56).

2. In widen_pkt_pointer_keeps_valid_bytes(), reg->range becomes 62 at the
backedge while the loop header checkpoint has rold->range = 64. In
bpf_is_state_visited() -> regsafe(), rold->range (64) > rcur->range (62)
prevents state pruning at label 1, and the second pass rejects the 8-byte
read at r6 + 0 because umax (56) + 8 > 62.

3. Conversely, if entry_reg->range is 0 at loop entry and the loop body
performs a pkt_end check in iteration 1 (setting reg->range =
reg_umax(r6) + N) before advancing r6 by a positive step,
bpf_clamp_scev_regs() at the backedge clamps reg->r64 back down to
bounds.r64 while keeping the loop body's reg->range, inflating the valid
bytes (reg->range - reg_umax(reg)) in iteration 2 and permitting
out-of-bounds packet accesses (and similarly leaving negative AT_PKT_END
or BEYOND_PKT_END marks from the loop body intact).

Should bpf_clamp_scev_regs() reset reg->range to max(entry_reg->range, 0)
before calling bpf_set_reg_range()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027201436.3192716-1-alexei.starovoitov@gmail.com?part=2

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf 1/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution
  2026-10-27 20:14 ` [PATCH bpf 1/2] " Alexei Starovoitov
  2026-10-27 20:59   ` bot+bpf-ci
@ 2026-10-27 21:04   ` sashiko-bot
  1 sibling, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-27 21:04 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] bpf: out-of-bounds packet range in sync_widened_reg() when r64 crosses zero
- [Critical] bpf: missing range reset in bpf_clamp_scev_regs() corrupts packet range
- [High] bpf: verifier_bug() in bpf_reg_union() when sync_widened_reg() mutates range
--

commit f227a7637607cf60f782fa9321486ac435f7a53c
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Keep the valid bytes of a packet pointer in scalar evolution

This patch updates sync_widened_reg() in bpf_set_reg_range() and
bpf_reg_union() to reduce reg->range when reg_bounds_sync() lowers umax on a
widened packet pointer, and clears negative pkt_end marks in
bpf_widen_scev_regs().

> diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c
> index 04d4962448e27..f28eddd17d78f 100644
> --- a/kernel/bpf/scev.c
> +++ b/kernel/bpf/scev.c
> @@ -2838,6 +2838,9 @@ int bpf_widen_scev_regs(struct bpf_verifier_env *env, struct bpf_verifier_state
>  				bpf_log(log, "\n");
>  			}
>  			scratch_widened_reg_id(env, reg);
> +			/* "at pkt_end" and "beyond pkt_end" are not valid in all iterations */
> +			if (type_is_pkt_pointer(reg->type))
> +				reg->range = max(reg->range, 0);
>  			err = bpf_set_reg_range(env, reg, bounds.r64, bounds.r32,
>  						bounds.base, bounds.step);

[Severity: Critical]
Should bpf_clamp_scev_regs() also reset reg->range from entry_reg (i.e.
max(entry_reg->range, 0)) before calling bpf_set_reg_range()?

On every loop backedge, do_check() -> handle_loop_entry_exit() ->
maybe_clamp_scev_regs() -> bpf_clamp_scev_regs() recomputes bounds from the
pre-widen loop-entry register entry_reg and calls bpf_set_reg_range() ->
sync_widened_reg(reg) on the current state's reg without resetting
reg->range:

kernel/bpf/scev.c:bpf_clamp_scev_regs() {
    ...
		linear_bounds(entry_reg, max_header_count, &coefs, &bounds);
		...
		scratch_widened_reg_id(env, reg);
		err = bpf_set_reg_range(env, reg, bounds.r64, bounds.r32,
					bounds.base, bounds.step);
    ...
}

This appears to cause two regressions on loop backedges:

1. Stale loop-body range leading to out-of-bounds packet access:
Suppose before a 2-iteration loop r6 = pkt has r64 = [0, 0] and range = 0
(widened at the header to r64 = [0, 8], range = 0), and inside the loop body
r6 += 8 advances r6 to r64 = [8, 16] followed by if r6 > pkt_end goto exit.
find_good_pkt_pointers() sets r6->range = reg_umax(r6) = 16 (representing 0
valid bytes past r6, since iteration 0 only proved pkt + 8 <= pkt_end).

When the backedge returns to the loop header, bpf_clamp_scev_regs() resets
r6->r64 back to [0, 8] (umax = 8) while leaving r6->range = 16 from the loop
body. If the backedge state is not pruned at the header (for example, when
another register or stack slot changes), iteration 1 executes at the header
with r6 having umax = 8 and range = 16 (8 valid bytes), allowing
*(u64 *)(r6 + 0) to read 8 bytes past pkt_end when r6 == pkt + 8 == pkt_end.
Similarly, negative AT_PKT_END or BEYOND_PKT_END marks set inside the loop
body are not cleared by bpf_clamp_scev_regs().

2. Repeated range subtraction preventing loop convergence:
When entry_reg has range = R > 0 before the loop and sync_widened_reg()
lowers umax by d = 2 (for instance, 58 -> 56), bpf_widen_scev_regs() at loop
entry sets the header checkpoint state's rold->range = R - 2. When the
backedge reaches the loop header, the current state's reg enters
bpf_clamp_scev_regs() with reg->range already equal to R - 2.

Because bpf_clamp_scev_regs() recomputes bounds.r64 = [0, 58] from entry_reg
and calls bpf_set_reg_range() -> sync_widened_reg(reg), umax drops from 58
to 56 a second time and sync_widened_reg() subtracts 2 again, setting
rcur->range = R - 4. In kernel/bpf/states.c:regsafe(), rold->range >
rcur->range returns false, preventing state pruning at the loop header and
shrinking reg->range on each pass.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index a0a5f075d2018..66d008166f169 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17768,6 +17768,20 @@ bool bpf_same_memory_origin(const struct bpf_reg_state *reg_a,
>  	}
>  }
>  
> +/*
> + * 'range - umax' bytes from a packet pointer are in the packet. reg_bounds_sync() can make umax
> + * smaller: with a step of 8, or with 3 zero bits in var_off, 58 becomes 56. Make the range
> + * smaller by the same number, so that the number of bytes does not grow.
> + */
> +static void sync_widened_reg(struct bpf_reg_state *reg)
> +{
> +	u64 umax = reg_umax(reg);
> +
> +	reg_bounds_sync(reg);
> +	if (type_is_pkt_pointer(reg->type) && reg->range > 0)
> +		reg->range = max_t(s64, reg->range - (s64)(umax - reg_umax(reg)), 0);
> +}

[Severity: Critical]
Can sync_widened_reg() corrupt reg->range when reg->r64 spans negative and
non-negative offsets (smin < 0 && smax >= 0)?

In kernel/bpf/scev.c:linear_bounds(), when a packet pointer is decremented in
a loop (slope < 0) or starts at a negative offset before the loop,
bounds.r64 = cnum64_from_srange(min_val, max_val) can have min_val < 0 and
max_val >= 0.

In kernel/bpf/cnum_defs.h:cnum64_umax():

ut FN(umax)(struct cnum_t cnum)
{
	return FN(urange_overflow)(cnum) ? UT_MAX : cnum.base + cnum.size;
}

any cnum64 that crosses the U64_MAX/0 boundary returns U64_MAX from
reg_umax(reg). Using reg_umax(reg) in sync_widened_reg() leads to two cases
where check_packet_access() can allow out-of-bounds packet reads or writes
past pkt_end:

1. Range inflation when reg_bounds_sync() rounds smin up to >= 0:
Suppose before a 2-iteration loop:

    r4 &= 0x38
    if r4 < 2 goto exit
    if r4 > 50 goto exit
    r6 = pkt
    r6 += r4
    r5 = r6
    r5 += 8
    if r5 > pkt_end goto exit

so r6 has r64 = [2, 50], var_off = (0; 0x38), range = 58. If the loop body
decrements r6 by 8 (r6 += -8), linear_bounds() produces r64 = [-6, 50],
base = 0, step = 8.

In sync_widened_reg(), umax before reg_bounds_sync(reg) is U64_MAX because
[-6, 50] crosses zero. Then reg_bounds_sync(reg) intersects r64 with step = 8
via __update_bounds_from_step(), narrowing r6->r64 to [0, 48] with
reg_umax(r6) = 48. As a result, (s64)(umax - reg_umax(reg)) evaluates to
(s64)(U64_MAX - 48) = -49, and reg->range - (s64)(umax - reg_umax(reg))
increases r6->range from 58 to 107 instead of reducing it to 56.

2. Failure to shrink range when smin remains < 0:
If the if r4 < 2 goto exit check is omitted above, linear_bounds() produces
r64 = [-8, 50], step = 8, and reg_bounds_sync(reg) tightens r6->r64 to
[-8, 48]. Because smin is still negative, reg_umax(reg) is U64_MAX both
before and after reg_bounds_sync(reg), so umax - reg_umax(reg) is 0 and
r6->range remains 58 instead of shrinking to 56. Inside the loop body,
r5 = r6; r5 += 8 yields r5->r64 = [0, 56] with r5->range = 58, allowing a
2-byte read at r5 + 0 past pkt_end when r4 = 48 and pkt_end = pkt + 56.

>  
>  int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>  		      struct cnum64 r64, struct cnum32 r32, u16 base, u16 step)
> @@ -17776,7 +17790,7 @@ int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>  	reg->step = step;
>  	reg->base = base;
>  	reg->var_off = tnum_unknown;
> -	reg_bounds_sync(reg); /* this should infer the tnum alignment */
> +	sync_widened_reg(reg); /* this should infer the tnum alignment */
>  	return reg_bounds_sanity_check(env, reg, "bpf_set_reg_range");
>  }
>  
> @@ -17821,7 +17835,7 @@ int bpf_reg_union(struct bpf_verifier_env *env, struct bpf_reg_state *acc,
>  		acc->base = base;
>  		acc->step = step;
>  	}
> -	reg_bounds_sync(acc);
> +	sync_widened_reg(acc);
>  	return reg_bounds_sanity_check(env, acc, "bpf_reg_union");
>  }

[Severity: High]
Does calling sync_widened_reg(acc) inside bpf_reg_union() trigger
verifier_bug(env, "union of incompatible registers") when union_any_reg()
iterates over multiple leaves?

In kernel/bpf/scev.c:union_any_reg(), bpf_reg_union(env, acc, leaf_reg) is
called in a loop for each leaf of an ANY expression. On each call,
bpf_reg_union() checks bpf_reg_union_compatible(acc, src):

kernel/bpf/verifier.c:bpf_reg_union_compatible() {
	if (a->type != b->type)
		return false;
	if (a->type == SCALAR_VALUE)
		return true;
	return !memcmp(a, b, offsetof(struct bpf_reg_state, var_off)) &&
	       a->id == b->id && a->parent_id == b->parent_id && a->map_uid == b->map_uid;
}

which compares a->range and b->range via memcmp().

For example, if at loop entry r6 has id = 1, r64 = [0, 50],
var_off = (0; 0x38), range = 58 and r5 = r6 + 8 has id = 1, r64 = [8, 58],
var_off = (0; 0x78), range = 58, and the loop body sets r5 = r6 so r5 has
SCEV (any r5 r6):

1. is_widenable_any() succeeds because both leaves have id = 1 and
   range = 58.
2. In union_any_reg(), the first leaf (entry_r5) is unioned into acc.
   sync_widened_reg(acc) lowers umax from 58 to 56 via var_off and changes
   acc->range from 58 to 56.
3. On the second leaf (entry_r6, which still has range = 58),
   bpf_reg_union_compatible(acc, entry_r6) sees acc->range (56) !=
   entry_r6->range (58), triggering verifier_bug(env, "union of incompatible
   registers") and returning -EFAULT.

In addition, calling sync_widened_reg(acc) on each leaf in union_any_reg()
can subtract umax reductions multiple times across intermediate leaves, and
the is_any() branch in bpf_widen_scev_regs() does not clear negative
pkt_end marks on reg->range before unioning.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027201436.3192716-1-alexei.starovoitov@gmail.com?part=1

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-27 21:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 20:14 [PATCH bpf 0/2] bpf: Keep the valid bytes of a packet pointer in scalar evolution Alexei Starovoitov
2026-10-27 20:14 ` [PATCH bpf 1/2] " Alexei Starovoitov
2026-10-27 20:59   ` bot+bpf-ci
2026-10-27 21:04   ` sashiko-bot
2026-10-27 20:14 ` [PATCH bpf 2/2] selftests/bpf: Test the valid bytes of a packet pointer in a counted loop Alexei Starovoitov
2026-10-27 20:59   ` bot+bpf-ci
2026-10-27 20:59   ` sashiko-bot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.