From 15123f84da7138f31d34b10444bd6fe245f4bbbd Mon Sep 17 00:00:00 2001 From: Martin Teichmann Date: Wed, 29 Oct 2025 11:58:28 +0100 Subject: [PATCH] bpf: tail calls do not modify packet data The bpf verifier checks whether packet data is modified within a helper function, and if so invalidates the pointer to that data. Currently the verifier always invalidates if the helper function called is a tail call, as it cannot tell whether the called function does or does not modify the packet data. However, in this case, the fact that the packet might be modified is irrelevant in the code following the helper call, as the tail call only returns if there is nothing to execute, otherwise the calling (sub)program will return directly after the tail call finished. So it is this (sub)program for which the pointer to packet data needs to be invalidated. Fortunately, there are already two distinct points in the code for invalidating packet pointers directly after a helper call, and for entire (sub)programs. This commit assures that the pointer is only invalidated in the relevant case. Note that this is a regression bug: taking care of tail calls only became necessary when subprograms were introduced, before commit 1a4607ffba35 using a packet pointer after a tail call was working fine, as it should. Fixes: 1a4607ffba35 ("bpf: consider that tail calls invalidate packet pointers") Signed-off-by: Martin Teichmann --- kernel/bpf/verifier.c | 3 ++- net/core/filter.c | 2 -- .../selftests/bpf/progs/verifier_sock.c | 18 ++++++++++++++++-- 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ff40e5e65c43..8d110d49ba17 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -17827,7 +17827,8 @@ static int visit_insn(int t, struct bpf_verifier_env *env) */ if (ret == 0 && fp->might_sleep) mark_subprog_might_sleep(env, t); - if (bpf_helper_changes_pkt_data(insn->imm)) + if (bpf_helper_changes_pkt_data(insn->imm) + || insn->imm == BPF_FUNC_tail_call) mark_subprog_changes_pkt_data(env, t); } else if (insn->src_reg == BPF_PSEUDO_KFUNC_CALL) { struct bpf_kfunc_call_arg_meta meta; diff --git a/net/core/filter.c b/net/core/filter.c index fa06c5a08e22..e1a1ff0a35f5 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -8032,8 +8032,6 @@ bool bpf_helper_changes_pkt_data(enum bpf_func_id func_id) case BPF_FUNC_xdp_adjust_head: case BPF_FUNC_xdp_adjust_meta: case BPF_FUNC_xdp_adjust_tail: - /* tail-called program could call any of the above */ - case BPF_FUNC_tail_call: return true; default: return false; diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c index 2b4610b53382..3e22b4f8aec4 100644 --- a/tools/testing/selftests/bpf/progs/verifier_sock.c +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c @@ -1117,10 +1117,10 @@ int tail_call(struct __sk_buff *sk) return 0; } -/* Tail calls invalidate packet pointers. */ +/* Tail calls in sub-programs invalidate packet pointers. */ SEC("tc") __failure __msg("invalid mem access") -int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk) +int invalidate_pkt_pointers_by_indirect_tail_call(struct __sk_buff *sk) { int *p = (void *)(long)sk->data; @@ -1131,4 +1131,18 @@ int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk) return TCX_PASS; } +/* Direct tail calls do not invalidate packet pointers. */ +SEC("tc") +__success +int invalidate_pkt_pointers_by_tail_call(struct __sk_buff *sk) +{ + int *p = (void *)(long)sk->data; + + if ((void *)(p + 1) > (void *)(long)sk->data_end) + return TCX_DROP; + bpf_tail_call_static(sk, &jmp_table, 0); + *p = 42; /* this is NOT unsafe: tail calls don't return */ + return TCX_PASS; +} + char _license[] SEC("license") = "GPL";