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 b88822d258 ("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: 6ccf49f793 ("bpf: Use SCEV to widen bounded loops")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
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

@@ -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);

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)

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");
 }