Re: [PATCH v5 bpf-next 1/4] bpf: properly verify tail call behavior

[Eduard Zingerman <eddyz87@gmail.com>]

On Tue, 2025-11-18 at 14:39 +0100, Martin Teichmann wrote:
> A successful ebpf tail call does not return to the caller, but to the
> caller-of-the-caller, often just finishing the ebpf program altogether.
> 
> Any restrictions that the verifier needs to take into account - notably
> the fact that the tail call might have modified packet pointers - are to
> be checked on the caller-of-the-caller. Checking it on the caller made
> the verifier refuse perfectly fine programs that would use the packet
> pointers after a tail call, which is no problem as this code is only
> executed if the tail call was unsuccessful, i.e. nothing happened.
> 
> This patch simulates the behavior of a tail call in the verifier. A
> conditional jump to the code after the tail call is added for the case
> of an unsucessful tail call, and a return to the caller is simulated for
> a successful tail call.
> 
> For the successful case we assume that the tail call returns an int,
> as tail calls are currently only allowed in functions that return and
> int. We always assume that the tail call modified the packet pointers,
> as we do not know what the tail call did.
> 
> For the unsuccessful case we know nothing happened, so we do not need to
> add new constraints.
> 
> This approach also allows to check other problems that may occur with
> tail calls, namely we are now able to check that precision is properly
> propagated into subprograms using tail calls, as well as checking the
> live slots in such a subprogram.
> 
> Fixes: 1a4607ffba35 ("bpf: consider that tail calls invalidate packet pointers")
> Link: https://lore.kernel.org/bpf/20251029105828.1488347-1-martin.teichmann@xfel.eu/
> Signed-off-by: Martin Teichmann <martin.teichmann@xfel.eu>
> ---

Acked-by: Eduard Zingerman <eddyz87@gmail.com>

[...]

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 098dd7f21c89..117a2b1cf87c 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[...]

> @@ -11970,6 +11979,25 @@ static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
>  		env->prog->call_get_func_ip = true;
>  	}
>  
> +	if (func_id == BPF_FUNC_tail_call) {

[...]

> +		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);
> +			clear_all_pkt_pointers(env);
> +			mark_reg_unknown(env, regs, BPF_REG_0);
> +			err = prepare_func_exit(env, &env->insn_idx);
> +			if (err)
> +				return err;
> +			env->insn_idx--;

This insn_idx adjustment is a bit unfortunate, but refactoring getting
rid of it is probably out of scope for this series.