From 88e2c896d8598a49da1e01f27ce23acd5dc04b32 Mon Sep 17 00:00:00 2001 From: Alexei Starovoitov Date: Fri, 30 Oct 2026 21:09:49 +0000 Subject: [PATCH] bpf: Keep the valid bytes of a packet pointer in scalar evolution 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 the largest offset. The largest offset is smax, not umax. With 'r6 += -8' in the loop the offset can be negative. Then umax is U64_MAX before and after reg_bounds_sync(). The verifier rejects an access through such r6. It accepts an access through 'r5 = r6; r5 += 8', and r5 has the range of r6. All registers of a union have the same range, and bpf_reg_union() makes sure of that for each register. Thus change the range one time, after the last register. The range goes with the largest offset of all registers. The range can also be a mark: the pointer is at pkt_end or beyond pkt_end. The mark is for the pointer at the entry of the loop, not for all iterations. Remove it when the pointer is widened. The arithmetic in the loop removes the mark too. Thus the state that returns to the head was not pruned, and the verifier made one more pass with the two branches of each compare. Now the first pass examines the two branches. The verdict can change only when no state returned to the head: the mark sent each path out of the loop, and the other insns of the loop were dead code. Fixes: 6ccf49f79327 ("bpf: Use SCEV to widen bounded loops") Signed-off-by: Alexei Starovoitov Link: https://patch.msgid.link/20261030210950.1904640-2-alexei.starovoitov@gmail.com Closes: https://lore.kernel.org/bpf/sashiko-outbox-187068@kernel.org/ --- include/linux/bpf_verifier.h | 1 + kernel/bpf/scev.c | 7 +++++++ kernel/bpf/verifier.c | 13 +++++++++++++ 3 files changed, 21 insertions(+) diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 7baeb831281f..8852c173c53e 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -1900,6 +1900,7 @@ bpf_global_subprog_stack_access_bytes(struct bpf_verifier_env *env, int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env); bool bpf_same_memory_origin(const struct bpf_reg_state *reg_a, const struct bpf_reg_state *reg_b); +void bpf_trim_pkt_range(struct bpf_reg_state *reg, s64 smax); 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); bool bpf_reg_union_compatible(const struct bpf_reg_state *a, const struct bpf_reg_state *b); diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c index 7b82fd79f9cc..1123a14e7389 100644 --- a/kernel/bpf/scev.c +++ b/kernel/bpf/scev.c @@ -2762,6 +2762,7 @@ static int union_any_reg(struct bpf_verifier_env *env, struct bpf_func_state *lo const struct bpf_reg_state *leaf_reg; struct scev *scev = env->scev; u32 l, r, leaf, order; + s64 smax = reg_smax(acc); s64 imm; int err; @@ -2785,10 +2786,13 @@ static int union_any_reg(struct bpf_verifier_env *env, struct bpf_func_state *lo verifier_bug(env, "scev ANY union has an unsupported leaf"); return -EFAULT; } + smax = max(smax, reg_smax(leaf_reg)); err = bpf_reg_union(env, acc, leaf_reg); if (err) return err; } + /* All leaves have the same range. It goes with the largest offset of all leaves. */ + bpf_trim_pkt_range(acc, smax); return 0; } @@ -2837,6 +2841,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 26a5ee0708a3..a90bc6c4033a 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -17736,6 +17736,18 @@ bool bpf_same_memory_origin(const struct bpf_reg_state *reg_a, } } +/* + * 'range - smax' bytes from a packet pointer are in the packet. reg_bounds_sync() can make smax + * smaller than @smax: 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. umax is not + * sufficient: it is U64_MAX when the offset can be negative. + */ +void bpf_trim_pkt_range(struct bpf_reg_state *reg, s64 smax) +{ + if (type_is_pkt_pointer(reg->type) && reg->range > 0) + reg->range = max_t(s64, reg->range - (smax - reg_smax(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) { @@ -17745,6 +17757,7 @@ int bpf_set_reg_range(struct bpf_verifier_env *env, struct bpf_reg_state *reg, reg->base = base; reg->var_off = tnum_unknown; reg_bounds_sync(reg); /* this should infer the tnum alignment */ + bpf_trim_pkt_range(reg, cnum64_smax(r64)); return reg_bounds_sanity_check(env, reg, "bpf_set_reg_range"); }