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

Eduard Zingerman
2026-03-23T18:42:14Z

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/ …