Re: [PATCH bpf v1 03/10] bpf: Reject tail calls directly from callback frames

["Kumar Kartikeya Dwivedi" <memxor@gmail.com>]

On Thu Sep 3, 2026 at 5:21 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The patch prevents bpf_tail_call within a callback to fix the 'callback unexpected regs' verifier bug, but misses that BPF_LD_ABS and BPF_LD_IND trigger the exact same bug via the same prepare_func_exit() path.
> --
>
> commit a25ccc5cdb72d9330414949c17ecac5156595510
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
>     bpf: Reject tail calls directly from callback frames
>
>     A tail call from a non-zero frame is modeled as a return from that frame,
>     synthesizing R0 and calling prepare_func_exit() for the taken branch.
>     When the current frame is a synchronous callback, prepare_func_exit()
>     enforces the callback return-value contract and marks R0 precise, triggering
>     the "callback unexpected regs" verifier warning during precision backtracking.
>     This patch rejects a tail call made directly by a callback before
>     constructing the inconsistent return state.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 7d8ddb1bee00c..f540279ff4abb 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -11228,6 +11228,17 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>>  		if (env->cur_state->curframe) {
>>  			struct bpf_verifier_state *branch;
>>
>> +			/*
>> +			 * A taken tail call is modeled as a return from the current
>> +			 * frame. A callback frame cannot be left that way because
>> +			 * prepare_func_exit() would apply its return contract to the
>> +			 * unknown R0 synthesized below. Stack-depth validation rejects
>> +			 * this construct anyway.
>> +			 */
>> +			if (cur_func(env)->in_callback_fn) {
>> +				verbose(env, "cannot tail call within callback\n");
>> +				return -EINVAL;
>> +			}
>
> [Severity: High]
> This isn't a bug introduced by this patch, but doesn't BPF_LD_ABS inside a
> subprog trigger the exact same bug via the same prepare_func_exit() path?
>
> Looking at check_ld_abs() in kernel/bpf/verifier.c:
>
>     if (env->cur_state->curframe) {
>         struct bpf_verifier_state *branch;
>
>         mark_reg_scratched(env, BPF_REG_0);
>         branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false);
>         if (IS_ERR(branch))
>             return PTR_ERR(branch);
>         mark_reg_known_zero(env, regs, BPF_REG_0);
>         err = prepare_func_exit(env, &env->insn_idx);
>         if (err)
>             return err;
>         env->insn_idx--;
>     }
>
> If a CAP_BPF user loads a SCHED_CLS program (where BPF_LD_ABS is allowed),
> invokes bpf_loop(), passes PTR_TO_CTX through the stack to the callback_ctx
> argument, loads the context into R6 inside the callback, and executes
> BPF_LD_ABS, it appears check_ld_abs() will synthesize R0 and call
> prepare_func_exit().
>
> Since the callback frame lacks instruction history for R0, won't precision
> backtracking fail and hit the exact same "callback unexpected regs" verifier
> warning? Does check_ld_abs() need a similar rejection for callbacks?
>

Has to be fixed separately.

>>  			mark_reg_scratched(env, BPF_REG_0);
>>  			branch = push_stack(env, env->insn_idx + 1, env->insn_idx, false);
>>  			if (IS_ERR(branch))