Re: [PATCH bpf-next v2 04/17] bpf: Prune verifier diagnostics on backtracking

[Eduard Zingerman <eddyz87@gmail.com>]

On Fri, 2026-06-19 at 22:59 +0200, Kumar Kartikeya Dwivedi wrote:
> Save the diagnostic event-log position with each verifier stack entry and
> reset the environment-owned stream together with the normal verifier log
> when a queued state is popped. Also reset the diagnostic stream after
> successful subprogram verification even when level-2 logging preserves the
> normal verifier log.
> 
> Record branch outcomes as true or false. Later reports use those
> events to show the path that reached a verifier error. For queued branches,
> append the queued outcome before push_stack() so the saved diagnostic
> position includes it, then reset back to the active path before continuing.
> 
> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
> ---

Nit: Let's not use the word "backtracking" as a subject for this
     patch, as in verifier it is easy to confuse with precision
     tracking backtracking.

>  kernel/bpf/verifier.c | 44 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 38 insertions(+), 6 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e81fdb0e22ae..ca4bba163418 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -192,6 +192,9 @@ struct bpf_verifier_stack_elem {
>  	struct bpf_verifier_stack_elem *next;
>  	/* length of verifier log at the time this state was pushed on stack */
>  	u32 log_pos;
> +	u64 diag_log_pos;

log_pos is u32, can we use u32 for diag_log_pos as well?

> +	bool diag_branch_valid;
> +	bool diag_branch_cond_true;

There is a 1 byte hole after 'st' and before 'insn_idx',
these two flags can fit there as bitfields.

Together this will remove the 6-byte padding and a 4-bytes hole from
this structure.

[...]

> @@ -1786,6 +1794,21 @@ static struct bpf_verifier_state *push_stack(struct bpf_verifier_env *env,
>  	return &elem->st;
>  }
>  
> +static struct bpf_verifier_state *
> +push_stack_with_branch_diag(struct bpf_verifier_env *env, int insn_idx,
> +			    int prev_insn_idx, bool speculative,
> +			    bool cond_true)
> +{
> +	struct bpf_verifier_state *st;
> +
> +	st = push_stack(env, insn_idx, prev_insn_idx, speculative);
> +	if (!IS_ERR(st)) {
> +		env->head->diag_branch_valid = true;
> +		env->head->diag_branch_cond_true = cond_true;

Tbh, I'd infer these flags instead of storing them.
If the state's prev_insn_idx is a conditional jump one can check if
this state's insn_idx is a target or a falltrhough. Would break for
'if ... goto +0' but I'd argue that it does not matter.
This would allow to move the

  bpf_diag_record_branch(env, head->prev_insn_idx, head->diag_branch_cond_true);

call from pop_stack to do_check(), near the:

  if (env->log.level & BPF_LOG_LEVEL2 && do_print_state) {
     verbose(env, "\nfrom %d to %d%s:", ...);
     ...
  }

and to make do with the activate_diag_branch flag in the pop_stack().

> +	}
> +	return st;
> +}
> +
>  static const char *reg_arg_name(struct bpf_verifier_env *env, argno_t argno)
>  {
>  	char *buf = env->tmp_arg_name;

[...]