Re: [PATCH v6 bpf-next 3/4] bpf: correct stack liveness for tail calls

Chris Mason
2025-12-12T02:07:12Z

On Wed, 19 Nov 2025 17:03:54 +0100 Martin Teichmann <martin.teichmann@xfel.eu> wrote:

> From: Eduard Zingerman <eddyz87@gmail.com>
>
> This updates bpf_insn_successors() reflecting that control flow might
> jump over the instructions between tail call and function exit, verifier
> might assume that some writes to parent stack always happen, which is
> not the case.
>

Hi everyone,

I ran recent upstream commits through claude and it flagged this one. Looks
like a different comment than it had made earlier:

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 9426367fc9113..0828718a8ba71 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -18156,6 +18160,25 @@ static int visit_gotox_insn(int t, struct bpf_verifier_env *env)
> return keep_exploring ? KEEP_EXPLORING : DONE_EXPLORING;
> }
>
> +static int visit_tailcall_insn(struct bpf_verifier_env *env, int t)
> +{
> + static struct bpf_subprog_info *subprog;
> + struct bpf_iarray *jt;
> +
> + if (env->insn_aux_data[t].jt)
> + return 0;
> +
> + jt = iarray_realloc(NULL, 2);
> + if (!jt)
> + return -ENOMEM;
> +
> + subprog = bpf_find_containing_subprog(env, t);
> + jt->items[0] = t + 1;
> + jt->items[1] = subprog->exit_idx;
> + env->insn_aux_data[t].jt = jt;
> + return 0;
> +}
> +

[ ... ]

> @@ -18216,6 +18239,8 @@ static int visit_insn(int t, struct bpf_verifier_env *env)
> mark_subprog_might_sleep(env, t);
> if (bpf_helper_changes_pkt_data(insn->imm))
> mark_subprog_changes_pkt_data(env, t);
> + if (insn->imm == BPF_FUNC_tail_call)
> + visit_tailcall_insn(env, t);
^^^^^^^^^^^^^^^^^^^^^^^

The return value of visit_tailcall_insn() is not checked here. If
iarray_realloc() fails and returns -ENOMEM, the error is silently
discarded. This is inconsistent with visit_gotox_insn() which properly
returns its value.

Under memory pressure, the jt allocation fails silently, leaving
env->insn_aux_data[t].jt as NULL. Later, bpf_insn_successors() will
not return the expected successors for the tail call, and the stack
liveness fix this patch implements would not take effect.

Should this be something like:

if (insn->imm == BPF_FUNC_tail_call) {
ret = visit_tailcall_insn(env, t);
if (ret < 0)
return ret;
}

> + } else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) {

[ ... ]