Re: [PATCH bpf-next v2 05/13] bpf: keep the range across a sign extension that cannot change it

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

On 9/12/26 12:02 PM, Alexei Starovoitov wrote:
> On Thu Sep 10, 2026 at 9:46 AM PDT, Vineet Gupta wrote:
>> coerce_reg_to_size_sx() decides whether a sign extension is lossless by
>> comparing the bits above the field in smin and smax:
>>
>> 	top_smax_value = ((u64)reg_smax(reg) >> num_bits) << num_bits;
>> 	top_smin_value = ((u64)reg_smin(reg) >> num_bits) << num_bits;
>> 	if (top_smax_value != top_smin_value)
>> 		goto out;
>>
>> Equal high bits do imply the truncation is lossless, but the converse does
>> not hold. Whenever the range straddles zero the high bits necessarily
>> differ -- smin sign-extends to all ones, smax to all zeroes -- even when
>> every value in the range fits the field and (sN)v == v throughout. The
>> second gate, "both of s64_max/s64_min positive or negative", rejects the
>> same shape again for the same reason.
>>
>> So a register holding an errno-or-zero value, [-4095, 0], comes out of r0
>> = (s32)r0 as the full [S32_MIN, S32_MAX] even though the instruction is a
>> no-op on it. The no_sext test at the call site does not help: it is an
>> unsigned check, so it only covers non-negative values that fit.
>>
>> Test the range against the field directly and return early when it fits.
>> Sign extension is then the identity, so nothing needs updating -- which
>> also preserves var_off, where the existing path would have replaced known
>> bits with a coarse tnum_range().
>>
>> This only tightens: the early return fires exactly where the value is
>> provably unchanged, and the cases the current tests do accept still take
>> the same path and produce the same bounds.
>>
>> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
>> ---
>> v2: new. coerce_reg_to_size_sx() is fixed rather than special-cased,
>> which is what was asked on RFC 5/6; it removes the RFC's
>> call-then-overwrite at the mov site.
>>
>>   kernel/bpf/verifier.c | 22 ++++++++++++++++++++++
>>   1 file changed, 22 insertions(+)
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 58e788f53ae5..eb093194e2a3 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -5754,6 +5754,7 @@ static void set_sext64_default_val(struct bpf_reg_state *reg, int size)
>>   static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size)
>>   {
>>   	s64 init_s64_max, init_s64_min, s64_max, s64_min, u64_cval;
>> +	s64 field_smin, field_smax;
>>   	u64 top_smax_value, top_smin_value;
>>   	u64 num_bits = size * 8;
>>   
>> @@ -5773,6 +5774,27 @@ static void coerce_reg_to_size_sx(struct bpf_reg_state *reg, int size)
>>   		return;
>>   	}
>>   
>> +	if (size == 1) {
>> +		field_smin = S8_MIN;
>> +		field_smax = S8_MAX;
>> +	} else if (size == 2) {
>> +		field_smin = S16_MIN;
>> +		field_smax = S16_MAX;
>> +	} else {
>> +		/* size == 4 */
>> +		field_smin = S32_MIN;
>> +		field_smax = S32_MAX;
>> +	}
>> +
>> +	/*
>> +	 * The range already fits the field, so (sN)v == v for every value the
>> +	 * register can hold and the sign extension changes nothing. The tests
>> +	 * below cannot reach this case once smin is negative: a negative smin
>> +	 * and a non-negative smax never share their high bits.
>> +	 */
>> +	if (reg_smin(reg) >= field_smin && reg_smax(reg) <= field_smax)
>> +		return;
>> +
> This patch can be a patch 1, since it looks unrelated to the rest?
> Would be good to test it individually. veristat run before/after.

OK, and the corresponding patch moves too.

Thx,
-Vineet