Re: [PATCH bpf-next v6 06/21] bpf: Make exception landing pads reachable in the CFG

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: DFS traversal invariant violation in visit_func_call_insn
--

commit 9f815c7b6f7049f1604884b98686a7516748e963
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Make exception landing pads reachable in the CFG

A bpf_unwind() or a bpf2bpf call inside the [begin_off, end_off) range of a
cleanup record can reach that record's landing pad. Add that edge to the
CFG walk, which explores the pad and makes both ends prune points, and to
bpf_insn_successors(), which liveness and the SCC passes walk.

> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index b0bd9ba951dfb..4e2b6985bc964 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c

[ ... ]

> @@ -160,17 +161,64 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
>  	return DONE_EXPLORING;
>  }
>  
> +static int visit_cleanup_pad_edge(int t, struct bpf_verifier_env *env)
> +{
> +	int *insn_stack = env->cfg.insn_stack;
> +	int *insn_state = env->cfg.insn_state;
> +	int w;
> +
> +	if (!env->cleanup_info_cnt)
> +		return DONE_EXPLORING;
> +	w = bpf_exc_pad_of_call(env, t);
> +	if (w < 0)
> +		return DONE_EXPLORING;
> +
> +	/*
> +	 * @t is a call that may branch here, and @w is the target of that
> +	 * branch, so both are prune points. @w especially: every covered call
> +	 * site in a region unwinds to the same pad, and without a prune point
> +	 * at its head the verifier walks the pad again for each of them.
> +	 */
> +	mark_prune_point(env, t);
> +	mark_prune_point(env, w);
> +	mark_jmp_point(env, w);
> +	mark_jump_target(env, w);
> +
> +	if (insn_state[w])
> +		return DONE_EXPLORING;
> +	if (env->cfg.cur_stack >= env->prog->len)
> +		return -E2BIG;
> +	insn_stack[env->cfg.cur_stack++] = w;
> +	insn_state[w] |= DISCOVERED;

[Severity: Medium]
Does setting the DISCOVERED flag here in visit_cleanup_pad_edge() mark the
pad prematurely?

By marking it DISCOVERED before fully exploring its path, any valid
cross-edges to the cleanup pad (e.g., from shared cleanup code) might be
incorrectly classified as back-edges and rejected by the verifier.

> +	return KEEP_EXPLORING;
> +}
> +

[ ... ]

>  static int visit_func_call_insn(int t, struct bpf_insn *insns,
>  				struct bpf_verifier_env *env,
>  				bool visit_callee)
>  {
> -	int ret, insn_sz;
> +	int ret, insn_sz, pad_ret;
>  	int w;
>  
> +	pad_ret = visit_cleanup_pad_edge(t, env);
> +	if (pad_ret < 0)
> +		return pad_ret;
> +
>  	insn_sz = bpf_is_ldimm64(&insns[t]) ? 2 : 1;
>  	ret = push_insn(t, t + insn_sz, FALLTHROUGH, env);

[Severity: Medium]
Is it safe to push the fallthrough edge even if visit_cleanup_pad_edge()
has just returned KEEP_EXPLORING?

The CFG checker typically relies on returning KEEP_EXPLORING immediately
after pushing a new node to maintain the invariant that DISCOVERED nodes are
strictly ancestors. By ignoring whether pad_ret is KEEP_EXPLORING and
eagerly pushing the fallthrough edge, this leaves the cleanup pad marked
DISCOVERED while the fallthrough path is being explored.

Could this lead to loop-free programs being incorrectly rejected if a
cross-edge on the fallthrough path is misidentified as a back-edge?

>  	if (ret)
> -		return ret;
> +		return merge_visit_ret(pad_ret, ret);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260926050006.2213110-1-yonghong.song@linux.dev?part=6