Re: [PATCH bpf-next v3 08/12] bpf: Recover the low 32 bits on the reverse link direction

[Vineet Gupta <vineet.gupta@linux.dev>]

On 10/20/26 10:20 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] bpf: missing contradiction check in sync_linked_regs() reverse-direction arm
> --
>
> commit 651910c4895680e635057947cc88b56f4edbf401
> Author: Vineet Gupta <vineet.gupta@linux.dev>
>
> bpf: Recover the low 32 bits on the reverse link direction
>
> This commit updates sync_linked_regs() in the BPF verifier to recover the low
> 32-bit bounds and subreg tnum when propagating from a narrow linked register
> (BPF_REG_LINK_ZEXT or BPF_REG_LINK_SEXT) back to a linked register wider than
> 32 bits.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 4621b476b206e..ac50252cec1f7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18785,29 +18785,54 @@ static void sync_linked_regs(struct bpf_verifier_env *env, struct bpf_verifier_s
>>   		if (reg->id != known_reg->id)
>>   			continue;
>>   		/*
>> -		 * The reverse direction does not work for a narrow known_reg:
>> -		 * it knows only its low 32 bits, which say nothing about a
>> -		 * wider reg's high half. Neither (u32)(base + delta) nor
>> -		 * (s32)(base + delta) inverts once the base is wider than 32
>> -		 * bits.
>> +		 * The reverse direction, from a narrow known_reg to a wider
>> +		 * reg, does not invert: neither (u32)(base + delta) nor
>> +		 * (s32)(base + delta) says anything about the base's high
>> +		 * half. It does still pin the low half, though -- both sides
>> +		 * are (base + delta) mod 2^32, differing only by their deltas
>> +		 * -- so recover that much and leave the high half alone.
>>   		 */
>>   		if ((known_reg->link_kind == BPF_REG_LINK_ZEXT ||
>>   		     known_reg->link_kind == BPF_REG_LINK_SEXT) &&
>>   		    reg->link_kind != BPF_REG_LINK_ZEXT &&
>>   		    reg->link_kind != BPF_REG_LINK_SEXT &&
>> -		    get_reg_width(reg) > 32)
>> -			continue;
>> +		    get_reg_width(reg) > 32) {
>> +			/*
>> +			 * Only @low's 32-bit half is meaningful: its var_off
>> +			 * is advanced over all 64 bits while r64 and the
>> +			 * base/step line are not, and nothing below reads
>> +			 * either. It is a scratch value, not a register state.
>> +			 */
>> +			struct bpf_reg_state low = *known_reg;
>> +			struct tnum low32;
>> +
>> +			fake_reg.type = SCALAR_VALUE;
>> +			__mark_reg_known(&fake_reg,
>> +					 (s64)reg->delta - (s64)known_reg->delta);
>> +			scalar32_min_max_add(&low, &fake_reg);
>> +			low.var_off = tnum_add(low.var_off, fake_reg.var_off);
>> +
>> +			cnum32_intersect_with(&reg->r32, low.r32);
>> +			low32 = tnum_intersect(tnum_subreg(reg->var_off),
>> +					       tnum_subreg(low.var_off));
>> +			reg->var_off = tnum_with_subreg(reg->var_off, low32);
>> +			reg_bounds_sync(reg);
>> +			goto scratched;
>> +		}
> [Severity: Medium]
> Can this reverse-direction arm leave reg with an empty range (CNUM32_EMPTY or
> CNUM64_EMPTY) or conflicting constant subreg bits when the low-32 bounds,
> var_off, or step progression of reg and known_reg contradict on an
> unreachable path?

Yeah bpf-ci bot asked the same and that is answered separately. What you 
mentioned is 4 ways, let me reply each below.

> In the sibling differencing arm further down in sync_linked_regs(), the code
> saves prev = *reg before intersecting and restores *reg = prev if
> range_bounds_violation(reg) holds:
>
> kernel/bpf/verifier.c:sync_linked_regs() {
> 	...
> 	cnum64_intersect_with(&reg->r64, prev.r64);
> 	cnum32_intersect_with(&reg->r32, prev.r32);
> 	reg->var_off = tnum_intersect(reg->var_off, prev.var_off);
> 	if (range_bounds_violation(reg))
> 		*reg = prev;
> 	reg_bounds_sync(reg);
> }
>
> In the new arm above, cnum32_intersect_with(&reg->r32, low.r32) and
> tnum_intersect() are applied directly without saving prev or checking for
> contradictions. If reg->r32 becomes CNUM32_EMPTY, reg_bounds_sync() exits
> early on range_bounds_violation(reg), and even when cnum32_intersect_with()
> is non-empty, reg_bounds_sync() can still empty reg->r32 or reg->r64 via
> __update_reg_bounds(), __reg_deduce_bounds(), or __update_bounds_from_step()
> when the narrowed low 32 bits contradict reg's 64-bit bounds, subreg tnum, or
> base/step progression. Similarly, if tnum_subreg(reg->var_off) and
> tnum_subreg(low.var_off) do not overlap (!tnum_overlap()), tnum_intersect()
> ORs conflicting constant bits together.
>
> Contradictions can reach this arm in several ways during check_cond_jmp_op()
> -> sync_linked_regs():
>
> 1. Mixed link kinds: if r6 is a wide unknown scalar (BPF_REG_LINK_NONE),
>     w7 = w6 (BPF_REG_LINK_ZEXT), and r8 = r6; r8 += 2 (BPF_REG_LINK_FULL),
>     narrowing r8 with if w8 != 7 goto out syncs r6->r32 to [5, 5] while
>     skipping r7 because r7->link_kind != r8->link_kind. A subsequent
>     if w7 != 9 goto out then enters the new arm for r6 and r8, intersecting
>     [5, 5] with [9, 9] and [7, 7] with [11, 11] to leave r6->r32 and r8->r32
>     as CNUM32_EMPTY.

Confirmed and this was the same as bpf-ci bot report.

> 2. Sign-extending mov (BPF_REG_LINK_SEXT): if a wide r6 has its low 32 bits
>     pinned by if w6 != 5 goto out (so r6->r32 is [5, 5]), r7 = (s32)r6 links
>     r7 with BPF_REG_LINK_SEXT while coerce_reg_to_size_sx() resets r7->r32 to
>     [S32_MIN, S32_MAX]. A following if w7 != 8 goto out enters the new arm
>     and intersects r6->r32 ([5, 5]) with [8, 8].

Confirmed.

     if w6 != 5 goto out    /* wide r6 gets r32 = [5, 5] */
     r7 = (s32)r6        /* SEXT */
     if w7 != 8 goto out    /* the new arm intersects [5,5] with [8,8] */

Note that since w6 is known to const 5, one would expect w7 to be a 
constant 5 as well:
coerce_reg_to_size_sx() decides whether the extension is a no-op from 
reg_smin()/reg_smax(), the 64-bit pair, which is wide here. So it takes 
set_sext64_default_val() and r7 comes out [S32_MIN, S32_MAX], unknown, 
and both branches are walked.

>
> 3. BPF_JSET fallthrough: in regs_refine_cond_op(), the BPF_JSET | BPF_X
>     fallthrough path calls __mark_reg_unbounded() before masking var_off.
>     After w7 = w6; if w6 == 0 goto out; (where r6->r32 is [1, U32_MAX]),
>     if w7 & -1 goto out refines r7 to 0 on fallthrough and syncs back to r6,
>     intersecting [1, U32_MAX] with [0, 0].

Confirmed.

     w7 = w6
     if w6 == 0 goto out    /* r6.r32 becomes [1, U32_MAX] */
     if w7 & -1 goto out    /* fallthrough refines r7 to 0 */
     r9 = 0
     r6 %= r9

All the 3 above are caught by the pre-sync check.

> 4. Step or tnum conflict inside reg_bounds_sync(): if a wide r6 has step > 1
>     (for example step == 3) and w7 = w6 resets r7->step to 1 via
>     zext_32_to_64(), narrowing w7 to a value in r6->r32 that is not a
>     multiple of 3 passes cnum32_intersect_with() and then becomes empty in
>     __update_bounds_from_step(r6) at the end of reg_bounds_sync().

This one is special as this is the only route where the contradiction is 
not visible at the point of intersection: cnum32_intersect_with() 
passes, and the range only empties later in __update_bounds_from_step().
This led to the final fix in v4: check both before and after sync.

Thx,
-Vineet

> When reg is left in this state, any subsequent instruction that checks it via
> reg_bounds_sanity_check() triggers verifier_bug() ("range bounds violation"),
> emitting a BPF_WARN_ONCE() and returning -EFAULT when
> BPF_F_TEST_REG_INVARIANTS is enabled.
>
> Should this arm save struct bpf_reg_state prev = *reg and restore *reg = prev
> if !tnum_overlap(tnum_subreg(reg->var_off), tnum_subreg(low.var_off)) or if
> range_bounds_violation(reg) is true after reg_bounds_sync(reg)?
>
> [ ... ]
>