Re: [PATCH bpf-next v6 07/21] bpf: Resume a covered call at its landing pad

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

On 9/25/26 10:15 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] bpf: verifier bypass of resource leaks on unwinding without pad
> - [High] bpf: double free via bpf_unwind_resume returning to normal path in verifier
> - [Medium] bpf: precision tracking corruption when backtracking through bpf_unwind_resume
> - [Medium] bpf: CFG and liveness graph mismatch for bpf_unwind_resume
> --
>
> commit c4ed3d378ed3f7a547e662e1b6885fb3e54fef13
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Resume a covered call at its landing pad
>
> This patch introduces the ability for a BPF program to resume execution at a
> landing pad after an unwinding operation. It updates the verifier's control
> flow and state management to correctly account for these transitions, treating
> the landing pad as an ordinary second successor in the same frame.
>
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index 0e38b9575328c..57665c67e66b3 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
> [ ... ]
>> @@ -434,8 +435,24 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   					return -EFAULT;
>>   			}
>>   
>> +			if (bpf_exc_pad_of_call(env, idx) == subseq_idx) {
>> +				/*
>> +				 * We came from this call's landing pad, which
>> +				 * runs in the caller's frame: on that path the
>> +				 * callee's frame was never entered, so there is
>> +				 * no frame to leave. The call clobbered r0-r5;
>> +				 * r6-r9 and the stack are the caller's own and
>> +				 * keep going back from here.
>> +				 */
>> +				bt_clear_reg(bt, BPF_REG_0);
>> +				if (bt_reg_mask(bt) & BPF_REGMASK_ARGS) {
>> +					verifier_bug(env, "landing pad unexpected regs %x",
>> +						     bt_reg_mask(bt));
>> +					return -EFAULT;
>> +				}
>> +				return 0;
> [Severity: Medium]
> Does the BPF_CALL backtracking logic correctly handle bpf_unwind_resume()?

No. See below.

>
> When backtracking from a caller's instruction into a callee's pad and
> encountering bpf_unwind_resume(), it looks like backtrack_insn() processes it
> as a standard call without transitioning the backtrack frame (like
> bt_subprog_enter() does for BPF_EXIT).
>
> Could this leave the backtrack frame in the caller's context while traversing
> the callee, leading to precision tracking corruption?

Yes. I missed handling function bpf_is_unwind_resume_kfunc() which will
provide bt_subprog_enter().

>
>>   			/* callx calls static subprogs only */
>> -			if (subprog >= 0 && bpf_subprog_is_global(env, subprog)) {
>> +			} else if (subprog >= 0 && bpf_subprog_is_global(env, subprog)) {
> [ ... ]
>> diff --git a/kernel/bpf/cfg.c b/kernel/bpf/cfg.c
>> index 4e2b6985bc964..cfd4fe4049cae 100644
>> --- a/kernel/bpf/cfg.c
>> +++ b/kernel/bpf/cfg.c
> [ ... ]
>> @@ -678,6 +687,8 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
>>   				mark_subprog_changes_pkt_data(env, t);
>>   			if (ret == 0 && bpf_is_throw_kfunc(insn))
>>   				mark_subprog_might_throw(env, t);
>> +			if (ret == 0 && bpf_is_unwind_kfunc(insn))
>> +				mark_subprog_might_unwind(env, t);
>>   		}
>>   		return visit_func_call_insn(t, insns, env, insn->src_reg == BPF_PSEUDO_CALL);
> [Severity: Medium]
> Is there a mismatch between the control flow graph and liveness tracking here?
>
> In visit_insn(), bpf_unwind_resume is processed as a standard kfunc call,
> which pushes a fallthrough edge into the CFG via visit_func_call_insn().
>
> However, in bpf_insn_successors() in liveness.c, it correctly acts as an exit:
>
>> diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
>> index 4e0273a8ceee4..c9ee4f10f7255 100644
>> --- a/kernel/bpf/liveness.c
>> +++ b/kernel/bpf/liveness.c
> [ ... ]
>> @@ -364,6 +364,9 @@ bpf_insn_successors(struct bpf_verifier_env *env, u32 idx)
>>   		return jt;
>>   	}
>>   
>> +	if (unlikely(bpf_is_unwind_resume_kfunc(insn)))
>> +		return succ;
>> +
>>   	opcode_info = &opcode_info_tbl[BPF_CLASS(insn->code) | BPF_OP(insn->code)];
> Could this mismatch, where the CFG considers the path reachable but liveness
> sees 0 successors, violate verifier graph invariants?

Okay, the below is an optimization:

+	if (unlikely(bpf_is_unwind_resume_kfunc(insn)))
+		return succ;

let me remove it so the number of successor's will be consistent between
cfg and liveness.

>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index fc3df452de2e4..77176250f866c 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>> @@ -19167,6 +19167,40 @@ enum {
>>   	INSN_IDX_UPDATED = 2,
>>   };
>>   
>> +static int push_cleanup_pad_branch(struct bpf_verifier_env *env, int insn_idx)
>> +{
>> +	struct bpf_verifier_state *branch;
>> +	struct bpf_func_state *frame;
>> +	int pad = bpf_exc_pad_of_call(env, insn_idx);
>> +
>> +	if (pad < 0)
>> +		return 0;
>> +	branch = push_stack(env, pad, insn_idx, false);
>> +	if (IS_ERR(branch))
>> +		return PTR_ERR(branch);
>> +	frame = branch->frame[branch->curframe];
>> +	/*
>> +	 * The state at that call with the caller-saved registers gone: the
>> +	 * callee's epilogue put r6-r9 and the stack back on the way out.
>> +	 */
>> +	clear_caller_saved_regs(env, frame->regs);
>> +	mark_reg_unknown(env, frame->regs, BPF_REG_0);
>> +	return 0;
>> +}
>> +
>> +static int process_bpf_unwind(struct bpf_verifier_env *env, int *insn_idx)
>> +{
>> +	struct bpf_func_state *frame = cur_func(env);
>> +	int pad = bpf_exc_pad_of_call(env, *insn_idx);
>> +
>> +	if (pad < 0)
>> +		return PROCESS_BPF_EXIT;
> [Severity: High]
> Could returning PROCESS_BPF_EXIT here bypass resource leak and lock checks?
>
> If a BPF program acquires a resource like a spinlock and calls bpf_unwind()
> without a covering landing pad, pad < 0 evaluates to true. The verifier then
> pops the state and terminates the branch without invoking check_resource_leak()
> as process_bpf_exit_full() would normally do.
>
> Since do_check_insn() evaluates this kfunc before the active_locks check, could
> this allow the program to unwind while holding a spinlock, causing a kernel
> memory leak or deadlock at runtime?

Right, check_resource_leak() is a must here.

>
>> +	clear_caller_saved_regs(env, frame->regs);
>> +	mark_reg_unknown(env, frame->regs, BPF_REG_0);
>> +	*insn_idx = pad;
>> +	return INSN_IDX_UPDATED;
>> +}
> [ ... ]
>> @@ -19404,6 +19438,20 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state)
>>   
>>   		env->jmps_processed++;
>>   		if (opcode == BPF_CALL) {
>> +			if (bpf_is_unwind_kfunc(insn))
>> +				return process_bpf_unwind(env, &env->insn_idx);
>> +			if (bpf_is_unwind_resume_kfunc(insn)) {
>> +				/*
>> +				 * Mark r0 a known zero -- unknown first, as
>> +				 * the known-zero helper keeps the type it
>> +				 * finds, which here is NOT_INIT. The fixups
>> +				 * lower this to 'r0 = 0; exit', so the frame
>> +				 * returns a real zero.
>> +				 */
>> +				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);
>> +			}
> [Severity: High]
> Could this simulate a normal return from the subprogram and lead to a double
> free vulnerability?
>
> Here, bpf_unwind_resume is handled by calling process_bpf_exit_full() with
> exception_exit = false, returning to the normal instruction after the call.
>
> However, earlier in push_cleanup_pad_branch(), the caller's landing pad branch
> was already pushed using the pre-call state.
>
> At runtime, the callee's pad actually jumps to the caller's pad, not the
> instruction after the call. If a callee's pad frees a resource, the verifier
> will check the caller's pad using the pre-call state (where the resource was
> still allocated).
>
> Could this allow the caller's pad to free the same resource a second time,
> resulting in a use-after-free or double free vulnerability?

Yes, it is possible.
	outer_call:				inner_call:
		bpf_rcu_read_lock
		call inner_call
						bpf_unwind
					landing_pad:
						bpf_rcu_read_unlock
						...
						bpf_unwind_resume
	landing_pad:
		bpf_rcu_read_unlock
		...

The outer_call landing_pad uses states at the beginning of 'call_inner_call'
which does have bpf_rcu_read_lock. So this caused double bpf_rcu_read_unlock.

To fix this issue, for every function call which has landing_pad,
checkpoint the entry state (bpf_rcu_read_lock, bpf_prompt_disable, etc.)
and right before bpf_unwind_resume to ensure the state is the same
as entry state. This will prevent the above double free.

>
>>   			if (env->cur_state->active_locks) {
>>   				/* similar to static subprog calls callx is allowed under a lock */
>>   				if (!bpf_is_callx(insn) &&