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

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

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

This issue is covered in previous patch. Indeed, check_resource_leak()
is needed.

>
> [ ... ]
>
>> @@ -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?

The above code is under branch:
	if (bpf_is_unwind_resume_kfunc(insn)) {
		...
                 mark_reg_unknown(env, cur_regs(env), BPF_REG_0);
                 mark_reg_known_zero(env, cur_regs(env), BPF_REG_0);
                 return process_bpf_exit_full(env, do_print_state, false);		
	}
So control is ready to return to either bpf prog (including main) or
exit the prog itself. This one (cur_func(env)->in_pad = false)
is irrelevant.

>
> 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.

This is possible. This is addressed in previous commit. The idea is
the state (related to resources) at the beginning of the function
must be the same as when bpf_unwind_resume() does resource checking.

>
>>   			}
> [ ... ]
>
>> @@ -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.
>
Yes, error_recoverable_with_nospec() should help to resolve the issue.

>> +		}
>> +