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

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-03-20 at 17:49 +0100, Paul Chaignon wrote:
> In a subsequent patch, the regs_refine_cond_op and reg_bounds_sync
> functions will be called in is_branch_taken instead of reg_set_min_max,
> to simulate each branch's outcome. Since they will run before we branch
> out, these two functions will need to work on temporary registers for
> the two branches.
> 
> This refactoring patch prepares for that change, by introducing the
> temporary registers on bpf_verifier_env and using them in
> reg_set_min_max.
> 
> This change also allows us to save one fake_reg slot as we don't need to
> allocate an additional temporary buffer in case of a BPF_K condition.
> 
> Finally, you may notice that this patch removes the check for
> "false_reg1 == false_reg2" in reg_set_min_max. That check was introduced
> in commit d43ad9da8052 ("bpf: Skip bounds adjustment for conditional
> jumps on same scalar register") to avoid an invariant violation. Given
> that "env->false_reg1 == env->false_reg2" doesn't make sense and
> invariant violations are addressed in a subsequent commit, this patch
> just removes the check.
> 
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Co-developed-by: Harishankar Vishwanathan <harishankar.vishwanathan@gmail.com>
> Signed-off-by: Harishankar Vishwanathan <harishankar.vishwanathan@gmail.com>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index b638ab841c10..fbc29fb96a60 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -17184,10 +17184,6 @@ static void regs_refine_cond_op(struct bpf_reg_state *reg1, struct bpf_reg_state
>   * but we don't support that right now.
>   */
>  static int reg_set_min_max(struct bpf_verifier_env *env,
> -			   struct bpf_reg_state *true_reg1,
> -			   struct bpf_reg_state *true_reg2,
> -			   struct bpf_reg_state *false_reg1,
> -			   struct bpf_reg_state *false_reg2,
>  			   u8 opcode, bool is_jmp32)
>  {
>  	int err;
> @@ -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/

[...]