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

[sashiko-bot@kernel.org]

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903144433.1716731-1-memxor@gmail.com?part=3