Re: [PATCH bpf-next 05/20] 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:
- [High] bpf: bpf_unwind_resume() retains fallthrough edge in CFG walk
--

commit 663c539069f6d8aab713a59c4d2aa7c90391da34
Author: Yonghong Song <yonghong.song@linux.dev>

bpf: Make exception landing pads reachable in the CFG

This commit updates the verifier's CFG walk and liveness analysis to account
for exception landing pads. It adds edges from calls within a cleanup record's
range to that record's landing pad. The commit message also states that
bpf_unwind_resume() reaches nothing after it and should have no successors.

> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
> index 842c7d1eabccc..9f8b8b54d5ea7 100644
> --- a/kernel/bpf/cfg.c
> +++ b/kernel/bpf/cfg.c
[ ... ]
> @@ -158,17 +159,57 @@ static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
[ ... ]
>  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: High]
Does this unconditionally push a fallthrough edge for all call instructions,
failing to account for non-returning kfuncs like bpf_unwind_resume()?

If the compiler places a bpf_unwind_resume() call at the very end of the
program (the end of the .text section) without any trailing instructions, will
the CFG walk attempt to push a fallthrough edge out of bounds and incorrectly
reject valid BPF programs with a 'jump out of range' error?

>  	if (ret)
> -		return ret;
> +		return merge_visit_ret(pad_ret, ret);
>  
>  	mark_prune_point(env, t + insn_sz);
[ ... ]
> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> index 44ecdc5b4ec2d..9cfd05f970bc9 100644
> --- a/kernel/bpf/liveness.c
> +++ b/kernel/bpf/liveness.c
[ ... ]
> @@ -256,6 +258,9 @@ bpf_insn_successors(struct bpf_verifier_env *env, u32 idx)
>  	succ = env->succ;
>  	succ->cnt = 0;
>  
> +	if (unlikely(bpf_is_unwind_resume_kfunc(insn)))
> +		return succ;
> +

[Severity: High]
This correctly applies the logic mentioned in the commit message (that
bpf_unwind_resume() reaches nothing after it and has no successors) to the
liveness algorithm. Should visit_func_call_insn() in kernel/bpf/cfg.c be
updated with similar logic to avoid pushing a fallthrough edge?

>  	opcode_info = &opcode_info_tbl[BPF_CLASS(insn->code) | BPF_OP(insn->code)];
>  	insn_sz = bpf_is_ldimm64(insn) ? 2 : 1;
>  	if (opcode_info->can_fallthrough)

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