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

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

On 9/28/26 5:31 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] bpf: landing pad state desync via discarded memory mutations allows OOB access
> --
>
> commit 41b3c5e14924fae7175095b7012838a14c2e5ffc
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Resume a covered call at its landing pad
>
> This commit allows a covered call to resume at its landing pad when bpf_unwind()
> is invoked by a subprogram. It pushes a cleanup pad branch with the caller's
> pre-call state and handles bpf_unwind() by returning PROCESS_BPF_EXIT for
> nested frames.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index fc3df452de2e4..ee074d4a936b5 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> [ ... ]
>
>> @@ -19167,6 +19186,57 @@ 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,
>> +			      bool *do_print_state)
>> +{
>> +	struct bpf_func_state *frame = cur_func(env);
>> +	int pad = bpf_exc_pad_of_call(env, *insn_idx);
>> +	int err;
>> +
>> +	if (pad < 0) {
>> +		err = check_resource_leak(env, false, !env->cur_state->curframe,
>> +					  "an unwind with no landing pad");
>> +		if (err)
>> +			return err;
>> +		if (env->cur_state->curframe)
>> +			return PROCESS_BPF_EXIT;
> [Severity: Critical]
> When process_bpf_unwind() returns PROCESS_BPF_EXIT here, the verifier stops
> exploring the callee and discards its state. Does this discard any memory
> mutations made by the callee to pointer arguments before calling bpf_unwind()?

You are right. It is my mistake. The key problem is here (in v7):

+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;
+}

esp.
   branch = push_stack(env, pad, insn_idx, false);
it ignored the state change in callee, e.g., a value in stack
may get changed. This will make verification incorrect due to
such changed value.

>
> Since the CPU unwinds the stack without undoing memory writes at runtime,
> could this desynchronize the verifier state from runtime state and allow
> out-of-bounds memory accesses?

Yes, see the above.

>
>> +		/*
>> +		 * The main program's frame returns at once, which is the
>> +		 * program returning. Mark r0 the zero the fixups leave after
>> +		 * the call, and leave through the exit, which is what holds
>> +		 * that zero to the program type.
>> +		 */
>> +		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);
>> +	}
>> +	clear_caller_saved_regs(env, frame->regs);
>> +	mark_reg_unknown(env, frame->regs, BPF_REG_0);
>> +	*insn_idx = pad;
>> +	return INSN_IDX_UPDATED;
>> +}
>> +
> [ ... ]
>
>> @@ -19421,7 +19491,29 @@ static int do_check_insn(struct bpf_verifier_env *env, bool *do_print_state)
>>   					return -EINVAL;
>>   				}
>>   			}
>> +			if (bpf_is_unwind_kfunc(insn) || bpf_is_unwind_resume_kfunc(insn)) {
>> +				err = check_kfunc_allowed_only(env, insn, env->insn_idx);
>> +				if (err)
>> +					return err;
>> +				if (bpf_is_unwind_kfunc(insn))
>> +					return process_bpf_unwind(env, &env->insn_idx,
>> +								  do_print_state);
>> +				/*
>> +				 * 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);
>> +			}
>>   			mark_reg_scratched(env, BPF_REG_0);
>> +			/* An unwind out of this call resumes at the pad. */
>> +			err = push_cleanup_pad_branch(env, env->insn_idx);
>> +			if (err)
>> +				return err;
> [Severity: Critical]
> push_cleanup_pad_branch() pushes a branch using a snapshot of the pre-call
> state. If the callee modified a stack variable (e.g., an array index) and then
> unwound the stack, does the verifier evaluate the landing pad with the original
> unmodified value?
>
> Could a BPF program use this to bypass bounds checks by mutating an index in
> the callee before unwinding, causing the verifier to optimize out the checks
> in the landing pad while using the out-of-bounds value at runtime?

You are right. See the above explanation. Will fix.

>
>>   			if (bpf_in_stack_arg_cnt(&env->subprog_info[cur_func(env)->subprogno]))
>>   				cur_func(env)->no_stack_arg_load = true;
>>   			if (bpf_is_callx(insn))