Re: [PATCH bpf-next v1 07/17] bpf: Track verifier context diagnostic history

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf_diag_history_start_idx anchors context diagnostic history at the innermost `enter` for nested contexts.
- [Medium] bpf_diag_history_start_idx completely suppresses context diagnostic history if the `enter` event is not found.
--

commit 6cd5658b83e6758ee70970a31217f30967606ec4
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Track verifier context diagnostic history

This commit records verifier context transitions in the diagnostic history.
Later reports can then anchor causal paths to the critical section that made
an operation invalid.

> diff --git a/kernel/bpf/diagnostics.c b/kernel/bpf/diagnostics.c
> index 56c1236ffe7ca..7a122c5025c27 100644
> --- a/kernel/bpf/diagnostics.c
> +++ b/kernel/bpf/diagnostics.c
[ ... ]
> @@ -893,8 +930,16 @@ static int bpf_diag_history_start_idx(const struct bpf_verifier_state *state,
>  		    event->kind == BPF_DIAG_HISTORY_REF_ACQUIRE &&
>  		    event->ref.ref_id == opts->ref_id)
>  			return i - 1;
> +		if (opts->scope == BPF_DIAG_HISTORY_SCOPE_CONTEXT &&
> +		    event->kind == BPF_DIAG_HISTORY_CONTEXT &&
> +		    event->ctx.enter &&
> +		    event->ctx.kind == opts->ctx_kind)
> +			return i - 1;

[Severity: Medium]
When traversing backwards, this loop stops at the first enter event it
encounters. For nested contexts, wouldn't this anchor the causal path at the
innermost context entry instead of the outermost entry that actually initiated
the restricted region? Is an event->ctx.depth == 1 check needed here?

>  	}
>  
> +	if (opts->scope == BPF_DIAG_HISTORY_SCOPE_CONTEXT)
> +		return state->diag_history_cnt;
> +

[Severity: Medium]
If the original enter event has been pushed out of the fixed-size history
buffer, returning state->diag_history_cnt skips the causal path rendering
entirely.

Looking at bpf_diag_print_history(), it will see start_idx equal to
state->diag_history_cnt and the loop will execute zero times. Furthermore,
because start_idx is not 0, it will bypass the omitted event warning:

	start_idx = bpf_diag_history_start_idx(state, opts);
	if (state->diag_history_omitted && start_idx == 0)
		verbose(env, "  ... %u earlier diagnostic events omitted ...

Should this fall through and return 0 instead of state->diag_history_cnt, so
the remaining events are still printed and the omitted warning is triggered?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260605063412.974640-1-memxor@gmail.com?part=7