Re: [PATCH bpf 1/2] bpf: fix REG INVARIANTS VIOLATION on speculative pointer arithmetic

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Thu Aug 20, 2026 at 8:25 PM CEST, Eduard Zingerman wrote:
> On Wed, 2026-08-19 at 20:58 +0800, Jiayuan Chen wrote:
>> Take the following unprivileged program as an example:
>>
>> 	r0 = bpf_map_lookup_elem(...)	/* PTR_TO_MAP_VALUE, offset 0 */
>> 	...
>> 	14: r0 += r1			/* r1 is a bounded scalar */
>> 	15: r9 = r0
>>
>> Loading it triggers a verifier warning from reg_bounds_sanity_check():
>>
>> 	verifier bug: REG INVARIANTS VIOLATION (alu): const subreg tnum out
>> 	of sync with range bounds r64={.base=0x0, .size=0x0}
>> 	r32={.base=0x0, .size=0xffffffff} var_off=(0x0, 0x0)
>>
>> What happens:
>>
>> 1. Processing insn 14 (r0 += r1) in adjust_ptr_min_max_vals(), the new
>>    offset is computed into dst_reg's var_off and 32/64-bit ranges.
>>
>> 2. Because pointer registers do not track 32-bit subregister bounds,
>>    __mark_reg32_unbounded() first sets r32 to the full range; r32 is
>>    re-derived from the offset at the end of the function by
>>    reg_bounds_sync().
>>
>> 3. On the unprivileged path, sanitize_ptr_alu() is called and, via
>>    sanitize_speculative_path() -> push_stack(), snapshots the current
>>    register state and schedules the next instruction (insn 15) to be
>>    verified directly as a speculative path.
>>
>> 4. That snapshot is taken between step 2 and the final reg_bounds_sync():
>>    at this point dst_reg's var_off still holds the (const) original
>>    offset while r32 has just been blanked to the full range, i.e. the two
>>    are out of sync. When the speculative path later verifies insn 15
>>    (r9 = r0), the inconsistent state reaches reg_bounds_sanity_check() and
>>    trips the warning.
>>
>> var_off and the 32-bit range must always be consistent. There are two
>> ways to keep the snapshot consistent:
>>
>>   1. sync var_off and r32 before the snapshot so they match, or
>>   2. leave r32 at its original (already consistent) value and blank it
>>      only after the snapshot.
>>
>> The whole point of sanitize_ptr_alu() is to insert a harmless masking
>> sequence that keeps the access in bounds under speculation, so the state
>> it snapshots should faithfully represent that. Take approach 2: move
>> __mark_reg32_unbounded() to after sanitize_ptr_alu(), so the speculative
>> snapshot keeps the pointer's original, consistent r32. The non-speculative
>> path is unchanged: r32 is still blanked before the offset is applied and
>> re-derived by reg_bounds_sync().
>>
>> Fixes: 5f99f312bd3b ("bpf: add register bounds sanity checks and sanitization")
>> Reported-by: Hiker Cl <clhiker365@gmail.com>
>> Closes: https://lore.kernel.org/bpf/CAGM=xGB1fJ9kT8XTitVo74B0WGqgjkoUHdLwzytwV0AyqeVApw@mail.gmail.com/
>> Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>> ---
>
> Acked-by: Eduard Zingerman <eddyz87@gmail.com>
>
>>  kernel/bpf/verifier.c | 11 ++++++++---
>>  1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index d17f14b35b79..d79038a8da10 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -14558,9 +14558,6 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, struct bpf_insn
>>  		return -EINVAL;
>>  	}
>>
>> -	/* pointer types do not carry 32-bit bounds at the moment. */
>> -	__mark_reg32_unbounded(dst_reg);
>> -
>>  	if (sanitize_needed(opcode)) {
>>  		ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg,
>>  				       &info, false);
>> @@ -14568,6 +14565,14 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env, struct bpf_insn
>>  			return sanitize_err(env, insn, ret);
>>  	}
>>
>> +	/* Pointer types do not carry 32-bit bounds at the moment. Blank r32
>> +	 * only after sanitize_ptr_alu() may have snapshotted dst_reg into a
>> +	 * speculative path: otherwise that snapshot freezes a const offset
>> +	 * with an unbounded r32, which later trips reg_bounds_sanity_check().
>> +	 * reg_bounds_sync() below re-derives r32 from the updated offset.
>> +	 */
>
> Nit: comment is a bit too verbose. "... otherwise reg_bounds_sanity_check()
>      might hit some constraints violations" should have been enough.
>

Fixed style and verbosity while applying.

>> +	__mark_reg32_unbounded(dst_reg);
>> +
>>  	switch (opcode) {
>>  	case BPF_ADD:
>>  		/*