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 <[email protected]> > --- > 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.
