Re: [PATCH v2 bpf-next 2/6] bpf: Use bpf_verifier_env buffers for reg_set_min_max

[Paul Chaignon <paul.chaignon@gmail.com>]

On Mon, Mar 23, 2026 at 11:42:11AM -0700, Eduard Zingerman wrote:
> On Fri, 2026-03-20 at 17:49 +0100, Paul Chaignon wrote:

[...]

> > @@ -17196,30 +17192,23 @@ static int reg_set_min_max(struct bpf_verifier_env *env,
> >  	 * variable offset from the compare (unless they were a pointer into
> >  	 * the same object, but we don't bother with that).
> >  	 */
> > -	if (false_reg1->type != SCALAR_VALUE || false_reg2->type != SCALAR_VALUE)
> > -		return 0;
> > -
> > -	/* We compute branch direction for same SCALAR_VALUE registers in
> > -	 * is_scalar_branch_taken(). For unknown branch directions (e.g., BPF_JSET)
> > -	 * on the same registers, we don't need to adjust the min/max values.
> > -	 */
> > -	if (false_reg1 == false_reg2)
> 
> A side note:
> 
> The above hunk was added as a part of [1] to mitigate some invariant
> violation errors. Surprisingly, none of the tests added in [1] fail
> on current master if above hunk is commented out. Probably due to
> recent improvements in bounds deduction. Should we remove these
> tests as a part of the series?
> 
> [1] https://lore.kernel.org/all/20251103063108.1111764-3-kafai.wan@linux.dev/

Nice catch! Out of those five new tests, the three "jset on same
register, scalar value unknown branch" never fail if you revert the
commit they were testing, even at the time they were added. I believe
these three tests were intended to cover the above "false_reg1 ==
false_reg2" check and supposed to fail with an invariant violation when
the check is missing.

I believe this check was never actually needed. For an invariant
violation to happen, we need regs_refine_cond_op to refine a register
based on a incorrectly-detected branch being verified. For jset, that
can only happen if one of the two registers is constant. In our case,
that would mean both registers are constant. But if both registers are
constant, then is_scalar_branch_taken is always able to precisely
deduce the outcome of the jset. Hence, we wouldn't even reach this
"false_reg1 == false_reg2" check.

I think I'll remove this check in a preparatory commit, along with the
related selftests and an explanation why it's all not-needed. Cc'ing
KaFai Wan in case I missed something.

> 
> [...]