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

[Yonghong Song <yonghong.song@linux.dev>]

On 9/25/26 10:21 PM, sashiko-bot@kernel.org wrote:
> 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.

The actual problem is below due to premature dfs stop's. See below.

>
>> +	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?

The above
	if (pad_ret < 0)
		return pad_ret;
is not correct and it may cause premature dfs stop (ignoring KEEP_EXPLORING).
The correct one is
	if (pad_ret != DONE_EXPLORING)
		return pad_ret;
This way, pad_ret can be either KEEP_EXPLORING or err.
If KEEP_EXPLORING, dfs can continue with landing pad.

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