Re: [PATCH bpf-next 2/8] bpf: Mark loop heads in check_cfg()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: stale CFG backedge metadata causes incorrect loop detection
--

commit de7e162d2062bfebc2e588cb33048e1edec24938
Author: Alexei Starovoitov <ast@kernel.org>

bpf: Mark loop heads in check_cfg()

check_cfg() finds back-edges while it walks the control flow graph.
Remember their targets as loop heads and which edge of the insn is
the back-edge. The main pass needs it to tell the state that enters
the loop from the state that went around it.
No functional change.

> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index 92f528c456052..5a08f079a4896 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -709,6 +709,9 @@ struct bpf_insn_aux_data {
>  	u32 non_stack_access:1; /* instruction can access non-stack memory */
>  	/* true if some jump or call instruction targets this instruction */
>  	u32 jump_target:1;
> +	u32 loop_head:1; /* target of a back-edge */
> +	u32 backedge_ft:1; /* the edge to the next insn is a back-edge */
> +	u32 backedge_br:1; /* the jump is a back-edge */

[Severity: High]
Does this new CFG metadata need to be cleared when dead conditional jumps are
rewritten?

When bpf_prune_dead_branches() in kernel/bpf/const_fold.c rewrites dead
conditional jumps into unconditional BPF_JA jumps:

kernel/bpf/const_fold.c:bpf_prune_dead_branches()
    *insn = BPF_JMP_A(taken ? insn->off : 0);
    changed = true;

it does not seem to clear the newly added backedge_br or backedge_ft metadata.

If a pruned branch was a back-edge but the actually taken branch is a forward
edge, and a later function like is_backedge() assumes unconditional jumps have
only a single edge:

kernel/bpf/states.c:is_backedge()
    if ((BPF_CLASS(insn->code) == BPF_JMP || BPF_CLASS(insn->code) == BPF_JMP32) &&
        BPF_OP(insn->code) == BPF_JA)
        return aux->backedge_ft || aux->backedge_br;

could this stale metadata cause forward edges to be incorrectly classified as
loop back-edges? This might lead to spurious state widening and the addition
of unnecessary may_goto loop guards.

>  	/*
>  	 * CFG strongly connected component this instruction belongs to,
>  	 * zero if it is a singleton SCC.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923223554.562379-1-alexei.starovoitov@gmail.com?part=2