Re: [PATCH bpf-next v4 07/20] bpf: Refuse exception cleanup shapes bpf_throw() cannot dispatch

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

On 9/21/26 2:20 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] bpf: kernel stack leak via >5 arg BPF-to-BPF calls in exception pads
> --
>
> commit 961251cb485953d7f6bd99ead3712d0e8268b8eb
> Author: Yonghong Song <yonghong.song@linux.dev>
>
> bpf: Refuse exception cleanup shapes bpf_throw() cannot dispatch
>
> This commit adds verification logic to reject specific control flow shapes
> that bpf_throw() and the exception cleanup handling cannot safely dispatch.
> It performs post-CFG checks using bpf_check_cleanup_exceptions() to inspect
> pad reachability, catch pads, and unwinding safety.
>
>> diff --git a/kernel/bpf/exception.c b/kernel/bpf/exception.c
>> index 67af78baa558d..da8fa6eb7e4b9 100644
>> --- a/kernel/bpf/exception.c
>> +++ b/kernel/bpf/exception.c
> [ ... ]
>
>> +static int cleanup_check_pad_insn(struct bpf_verifier_env *env, u32 i)
>> +{
>> +	struct bpf_insn *insn = &env->prog->insnsi[i];
>> +
>> +	if (bpf_helper_call(insn) && insn->imm == BPF_FUNC_tail_call) {
>> +		verbose(env,
>> +			"bpf_tail_call() at insn %u is in an exception cleanup landing pad\n",
>> +			i);
>> +		return -EINVAL;
>> +	}
>> +	/* A BPF_LD_[ABS|IND] can leave the frame through its epilogue. */
>> +	if (BPF_CLASS(insn->code) == BPF_LD &&
>> +	    (BPF_MODE(insn->code) == BPF_ABS || BPF_MODE(insn->code) == BPF_IND)) {
>> +		verbose(env,
>> +			"BPF_LD_[ABS|IND] at insn %u is in an exception cleanup landing pad\n",
>> +			i);
>> +		return -EINVAL;
>> +	}
>> +	if (is_stack_arg_st(insn) || is_stack_arg_stx(insn)) {
>> +		verbose(env,
>> +			"insn %u passes an on-stack call argument in an exception cleanup landing pad\n",
>> +			i);
>> +		return -EINVAL;
>> +	}
>> +	if (bpf_pseudo_kfunc_call(insn)) {
>> +		struct bpf_call_summary cs;
>> +
>> +		if (bpf_get_call_summary(env, insn, &cs) &&
>> +		    cs.arg_slot_cnt > MAX_BPF_FUNC_REG_ARGS) {
> [Severity: High]
> Does this omit checking BPF-to-BPF calls (bpf_pseudo_call) for on-stack
> arguments?
>
> The JIT executes exception cleanup landing pads on the unwinder's stack (rsp),
> while rbp points to the BPF frame. This check restricts kfuncs from passing
> arguments on the stack, but seems to miss BPF subprograms because
> bpf_get_call_summary() returns false for them.
>
> If a pad calls a BPF subprogram requiring more than 5 arguments, could the
> callee read uninitialized kernel stack memory belonging to the unwinder's
> caller?

We should be okay here. As the code explains, if the number of reg-level argument slots
more than 5, verifier will reject.

>> +			verbose(env,
>> +				"insn %u passes an on-stack call argument in an exception cleanup landing pad\n",
>> +				i);
>> +			return -EINVAL;
>> +		}
>> +	}
>> +	return 0;
>> +}
> [ ... ]
>
>> +static int cleanup_mark_pad_bodies(struct cleanup_ctx *ctx)
>> +{
>> +	struct bpf_verifier_env *env = ctx->env;
>> +	u32 i, sp = 0;
>> +	int ret;
> [ ... ]
>
>> +	while (sp) {
>> +		u32 j = ctx->stack[--sp];
>> +		enum cleanup_insn_kind kind;
>> +		int next, target, sub;
>> +		u32 start, end;
>> +
>> +		ret = cleanup_check_pad_insn(env, j);
>> +		if (ret)
>> +			return ret;
>> +
>> +		sub = cleanup_subprog_of(env, j);
>> +		start = env->subprog_info[sub].start;
>> +		end = env->subprog_info[sub + 1].start;
>> +		kind = cleanup_succ(env, j, start, end, &next, &target);
>> +
>> +		if (kind == CLEANUP_INSN_CALL) {
>> +			/* check_subprogs() registered every call target. */
>> +			int callee = cleanup_subprog_of(env, j + env->prog->insnsi[j].imm + 1);
>> +
>> +			if (env->subprog_info[callee].might_throw) {
> [Severity: High]
> Since cleanup_mark_pad_bodies() allows BPF-to-BPF calls in landing pads as
> long as they don't throw, should it also verify that the callee does not
> take on-stack arguments?
>
> For example, by checking if env->subprog_info[callee].arg_slot_cnt <=
> MAX_BPF_FUNC_REG_ARGS?
>
> Without this check, a subprogram requiring more than 5 arguments could be
> called, potentially leading to the kernel stack data leak mentioned above.

We should be okay here. Commit "bpf: Walk the exception unwind in the verifier",
we have

+static u32 unwind_pop_frame(struct bpf_verifier_env *env)
+{
+       struct bpf_verifier_state *state = env->cur_state;
+       struct bpf_func_state *callee = state->frame[state->curframe];
+       u32 callsite = callee->callsite;
+       struct bpf_func_state *caller;
+
+       caller = state->frame[state->curframe - 1];
+       account_processed_insns(env, callee, caller);
+       free_func_state(callee);
+       state->frame[state->curframe--] = NULL;
+       invalidate_outgoing_stack_args(env, caller);
+       return callsite;
+}

stack arguments will be invlidated, so once pop frame and go to
landing_pad code section, verification will fail due to un-initialized
stack arguments.

>
>> +				verbose(env,
>> +					"cleanup landing pad calls subprog %d at insn %u, which can throw while an exception is in flight\n",
>> +					callee, j);
>> +				return -EINVAL;
>> +			}
>> +		}
> [ ... ]
>