Re: [PATCH bpf-next 1/2] bpf: reject pointer operands for 32-bit recurrences/latches

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] bpf: 32-bit pointer recurrences not rejected in bpf_compute_loop_iters()
- [High] bpf: missing pointer and type checks for linear SCEV widening and clamping
--

commit ea9c83878323b0365b4124b8b3e8c881f6513530
Author: Eduard Zingerman <eddyz87@gmail.com>

bpf: reject pointer operands for 32-bit recurrences/latches

This commit updates scev_value_compatible() to reject pointer operands when
the loop latch uses a 32-bit comparison or a 32-bit recurrence, preventing
loop iteration counts from being inferred solely from pointer offsets.

> diff --git a/kernel/bpf/scev.c b/kernel/bpf/scev.c
> index fea77772ca967..44916071c16bb 100644
> --- a/kernel/bpf/scev.c
> +++ b/kernel/bpf/scev.c
> @@ -2024,7 +2024,8 @@ static void log_incompatible_scev_values(struct bpf_verifier_env *env, const cha
>  }
>  
>  static bool scev_value_compatible(struct bpf_verifier_env *env, struct bpf_func_state *st,
> -				  const struct scev_value *a, const struct scev_value *b)
> +				  const struct scev_value *a, const struct scev_value *b,
> +				  const struct linear_latch *latch)
>  {
>  	const struct bpf_reg_state *ra, *rb;
>  
> @@ -2035,7 +2036,7 @@ static bool scev_value_compatible(struct bpf_verifier_env *env, struct bpf_func_
>  	}
>  	ra = scev_reg_for_read(env, st, a->ptr_reg);
>  	rb = scev_reg_for_read(env, st, b->ptr_reg);
> -	if (ra && rb && bpf_same_memory_origin(ra, rb))
> +	if (ra && rb && bpf_same_memory_origin(ra, rb) && !latch->zext32 && !latch->jmp32)
>  		return true;

[Severity: Critical]
Checking !latch->zext32 && !latch->jmp32 in scev_value_compatible() only
rejects 32-bit pointer recurrences when the pointer register is one of the
latch comparison operands in compute_max_iters(). What happens if the loop
iteration count is bounded by a separate scalar latch, while another live
pointer register undergoes a 32-bit linear recurrence in the loop body (for
example, w1 = w1; w1 += 8)?

In kernel/bpf/scev.c, is_simple_linear() matches both 64-bit and 32-bit
linear recurrences:

static bool is_simple_linear(struct scev *scev, u32 id, struct linear_coefs *out)
{
	u32 base, slope;

	return is_linear_or_linear32(scev, id, &base, &slope, &out->zext32) &&
	       is_reg(scev, base, &out->base_reg) &&
	       is_imm(scev, slope, &out->slope) &&
	       out->slope <= S16_MAX &&
	       out->slope >= S16_MIN &&
	       out->slope != 0;
}

Then bpf_compute_loop_iters() accepts the register for widening because
is_widenable_reg_type(reg) allows pointer register types without checking
coefs.zext32:

		r_scev = header_env->reg2scev[r];
		reg = scev_reg_for_linear(cur_func, r);
		/* If SCEV for r is (linear <reg> <slope>) */
		if (reg && is_simple_linear(scev, r_scev, &coefs) && is_widenable_reg_type(reg)) {

At loop entry, bpf_widen_scev_regs() calls linear_bounds() on the pointer
register with coefs.zext32 set:

	if (coefs->zext32) {
		r32 = cnum32_from_cnum64(r64);
		r64 = cnum64_zext32(r32);

This truncates and zero-extends the pointer's tracked offset at 32 bits
while keeping reg->type as a pointer (for example, turning a negative stack
pointer offset such as [-16, -8] into [0xfffffff0, 0xfffffff8]).

Furthermore, when env->allow_ptr_leaks is enabled, 32-bit operations in the
loop body such as w1 = w1; w1 += 8 convert r1 into a scalar holding the
truncated 32-bit runtime address. When the backedge reaches the loop header,
bpf_clamp_scev_regs() computes linear_bounds() from entry_reg's pointer
offset and intersects that offset range with the backedge scalar in reg:

		linear_bounds(entry_reg, max_header_count, &coefs, &bounds);
		bounds.r64 = cnum64_intersect(reg->r64, bounds.r64);
		bounds.r32 = cnum32_intersect(reg->r32, bounds.r32);
		if (cnum64_is_empty(bounds.r64)) {
			verifier_bug(env, "scev clamp produced empty range for r%d", r);
			return -EFAULT;
		}

Can this either clamp the runtime-address scalar to the small pointer offset
range (allowing out-of-bounds memory access if the scalar is subsequently
added to a pointer) or produce an empty intersection and trigger
verifier_bug() if the scalar was narrowed in the loop body?

>  
>  incompatible:
> @@ -2267,7 +2268,7 @@ static bool compute_max_iters(struct bpf_verifier_env *env,
>  		}
>  		return false;
>  	}
> -	if (!scev_value_compatible(env, st, &initial_val, &bound_val))
> +	if (!scev_value_compatible(env, st, &initial_val, &bound_val, latch))
>  		return false;
>  
>  	initial = initial_val.value;

[Severity: High]
Related to the issue above, can a pointer register at loop entry also be
degraded to a scalar in the loop body even when coefs.zext32 is false,
causing bpf_clamp_scev_regs() to intersect a backedge scalar range with
loop-entry pointer offset bounds?

In kernel/bpf/scev.c:transfer(), a 64-bit native-endian byte swap (such as
r1 = le64 r1 on a little-endian host) sets need_bswap to false and op to 0,
treating the instruction as an identity operation in the expression graph:

			need_bswap = class == BPF_ALU64 || ((x_or_k == BPF_TO_LE) != little_endian);
			switch (insn->imm) {
			case 16: op = need_bswap ? BSWAP16 : ZEXT16; break;
			case 32: op = need_bswap ? BSWAP32 : ZEXT32; break;
			case 64: op = need_bswap ? BSWAP64 : 0; break;
			default:
				goto mark_dst_unknown;
			}

			id = reg2expr[dst];
			if (op)
				id = expr1(scev, op, reg2expr[dst]);

If a loop body executes r1 = le64 r1; r1 += 8 on a register that holds a
pointer at loop entry, transfer() produces a 64-bit linear recurrence with
coefs.zext32 == false, while check_alu_op() degrades r1 to an unbounded
scalar value.

When bpf_clamp_scev_regs() runs on the backedge, it does not check whether
entry_reg and reg have matching register types before clamping reg to the
bounds computed from entry_reg:

		entry_reg = scev_reg_for_linear(entry_st, r);
		reg = scev_reg_for_linear(cur_func_state, r);
		if (!entry_reg || !reg || entry_reg->type == NOT_INIT || reg->type == NOT_INIT ||
		    is_unbound_scev_reg(env, entry_st, r))
			continue;

		linear_bounds(entry_reg, max_header_count, &coefs, &bounds);
		bounds.r64 = cnum64_intersect(reg->r64, bounds.r64);

Should bpf_compute_loop_iters() reject coefs.zext32 on non-scalar registers,
and should transfer() or bpf_clamp_scev_regs() also ensure that a pointer
register at loop entry is not widened or clamped when degraded to a scalar
in the loop body?

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