Re: [PATCH bpf-next v2 07/17] bpf: Track verifier context diagnostic events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoded depth for IRQ contexts breaks nested diagnostic history anchoring.
--

commit 86418a115dc1ef24cac1a623674bfd700c69c32e
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Track verifier context diagnostic events

Record verifier context transitions in the diagnostic history to anchor
causal paths for lock, IRQ, RCU, and preempt regions. The depth of the
context is used to ensure nested regions correctly anchor at the outermost
active region.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 93941deb2cd8c..e584dec04b34d 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
[ ... ]
> @@ -1473,6 +1475,7 @@ static int acquire_irq_state(struct bpf_verifier_env *env, int insn_idx)
>  	s->id = ++env->id_gen;
>  
>  	state->active_irq_id = s->id;
> +	bpf_diag_record_context(env, insn_idx, BPF_DIAG_CONTEXT_IRQ, true, 1);

[Severity: Medium]
The commit message mentions using context depth to anchor nested regions at
the outer active region. Since BPF allows nested IRQ disables (for example,
via multiple bpf_local_irq_save calls), does hardcoding the depth to 1 here
break that behavior?

If a program nests IRQ contexts, multiple enter events will be recorded with
a depth of 1. When bpf_diag_history_context_start_idx() searches backwards
for an event with a depth of 1, won't it stop at the innermost IRQ enter
event rather than the outermost one?

>  	return s->id;
>  }

[ ... ]
> @@ -1552,6 +1561,9 @@ static int release_irq_state(struct bpf_verifier_state *state, int id)
>  		if (state->refs[i].id == id) {
>  			release_reference_state(state, i);
>  			state->active_irq_id = prev_id;
> +			bpf_diag_record_context(env, env->insn_idx,
> +						BPF_DIAG_CONTEXT_IRQ, false,
> +						state->active_irq_id ? 1 : 0);

[Severity: Medium]
Similarly, won't this hardcode the exit depth to 1 if there are still outer
nested IRQ contexts active? Should this calculate the actual number of
nested IRQ contexts instead?

>  			return 0;
>  		} else {
>  			prev_id = state->refs[i].id;

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