Re: [PATCH bpf-next v6 08/21] bpf: Refuse a landing pad that does not resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] bpf: bpf_unwind bypasses resource leak checks when no landing pad is present
- [High] bpf: bpf_unwind_resume creates invalid ghost paths by returning to caller's normal path
- [Medium] bpf: Speculative branches to landing pads bypass nospec recovery
--

commit 3aafeed4814ca53533c421881eb0f30857cc42a0
Author: Yonghong Song <yonghong.song@linux.dev>
Subject: bpf: Refuse a landing pad that does not resume

This commit restricts exception landing pads in BPF to only support cleanup
pads that resume via bpf_unwind_resume(). It adds checks in the verifier
to ensure that instructions are not reached both inside and outside a pad,
and it refuses unsupported program exits within a pad.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -19197,6 +19202,7 @@ static int process_bpf_unwind(struct bpf_verifier_env *env, int *insn_idx)
>  	int pad = bpf_exc_pad_of_call(env, *insn_idx);
>  
>  	if (pad < 0)
>  		return PROCESS_BPF_EXIT;

[Severity: Critical]
Does this early return in process_bpf_unwind() bypass resource leak checks?

If a program acquires a resource like a bpf_spin_lock and throws an
exception where there is no landing pad, it appears this directs the verifier
to pop the state without calling check_resource_leak(), allowing the
program to leak the lock at runtime.

[ ... ]

> @@ -19450,6 +19461,7 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state)
>  				 */
>  				mark_reg_unknown(env, cur_regs(env), BPF_REG_0);
>  				mark_reg_known_zero(env, cur_regs(env), BPF_REG_0);
> +				cur_func(env)->in_pad = false;
>  				return process_bpf_exit_full(env, do_print_state, false);

[Severity: High]
Could passing false to process_bpf_exit_full() here instruct the verifier to
continue at the callsite's normal return path instead of unwinding to the
caller's landing pad?

Because push_cleanup_pad_branch() eagerly pushed the caller's landing pad
with the pre-call state, returning to the normal success path might merge
the post-cleanup state incorrectly and cause a divergence between verifier
control flow and runtime execution.

>  			}

[ ... ]

> @@ -19578,6 +19590,12 @@ static int do_check(struct bpf_verifier_env *env)
>  			}
>  		}
>  
> +		if (unlikely(env->cleanup_info_cnt)) {
> +			err = bpf_exc_check_insn(env, insn);
> +			if (err)
> +				return err;

[Severity: Medium]
When do_check() returns an error from bpf_exc_check_insn() due to a
speculative visit to a landing pad, does it bypass the subsequent
error_recoverable_with_nospec() check entirely?

If the verifier speculatively explores a dead branch targeting a landing pad,
this might cause the verifier to fatally abort instead of properly recovering
by inserting a speculation barrier.

> +		}
> +

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