[PATCH bpf-next v2] bpf: Refuse the prologue for an extension if callers keep packet pointers

[PATCH bpf-next v2] bpf: Refuse the prologue for an extension if callers keep packet pointers

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

An extension that writes to the packet of a tc, lwt_xmit or sk_skb program
starts with bpf_unclone_prologue().  If the skb is cloned, that gives it
a new head.

It does not count as changing packet data.  changes_pkt_data goes by the
helpers and kfuncs that a program calls, and whether it gets a prologue
is known only after it has been compared with its target.  So the extension
can replace a global function that leaves the packet alone, and the callers
of that function keep their packet pointers across the call.  These point
into the old head, which is left to the other clone.  Once that is freed,
by the reader of a packet socket for example:

  BUG: KASAN: slab-use-after-free in bpf_prog_53bc21b9810cadbc_t_read+0x2e8/0x36e
  Read of size 1 at addr ffff888107bfe73c by task tt11/102
  Freed by task 112:
   kfree+0x161/0x500
   skb_release_data+0x2dc/0x340
   consume_skb+0xa6/0x1c0
   packet_recvmsg+0x4b3/0xa20

Nothing is wrong if the target program writes to the packet too: its own
prologue has uncloned the skb, and that of the extension does nothing.
Nor if the callers hold no packet pointers, like a dispatcher, or if the
main function is replaced.

Note for every global function that leaves the packet alone if it is
called while packet pointers, or slices of the dynptr of a packet, are
held.  What the callers of a global function hold is not known while it
is verified, so the functions that it calls are taken for such as well.
Refuse an extension with a prologue for such a function, unless the target
program has a prologue, both when the extension is loaded and when it
is attached to another target.

Functions can share a type id in func_info.  The first of them is the one
that is replaced, so look it up in one place.

Fixes: 7e40781cc8b7 ("bpf: verifier: Use target program's type for access verifications")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
v1 -> v2:
- Two functions with one type id got past the check: it took any of them,
  while the first one is replaced.  Same use-after-free, reproduced.
  bpf_check_attach_target() and bpf_check_ext_prologue() share the lookup
  now (sashiko, bpf-ci).

v1: https://lore.kernel.org/bpf/20261018012753.1724147-1-alexei.starovoitov@gmail.com/

 include/linux/bpf.h          |  2 ++
 include/linux/bpf_verifier.h |  2 ++
 kernel/bpf/fixups.c          |  6 ++++
 kernel/bpf/verifier.c        | 67 ++++++++++++++++++++++++++++++++----
 4 files changed, 71 insertions(+), 6 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index feb7de6483e3..54ae73e211f7 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1757,6 +1757,7 @@ struct bpf_func_info_aux {
 	bool unreliable;
 	bool called[BPF_FUNC_CTX_CNT];
 	bool verified[BPF_FUNC_CTX_CNT];
+	bool called_with_pkt_ptrs;
 };
 
 enum bpf_jit_poke_reason {
@@ -1894,6 +1895,7 @@ struct bpf_prog_aux {
 	bool jits_use_priv_stack;
 	bool priv_stack_requested;
 	bool changes_pkt_data;
+	bool has_prologue;
 	bool might_sleep;
 	bool kprobe_write_ctx;
 	struct {
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 7a72bb06fe56..951a90458d18 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -1381,6 +1381,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			    const struct bpf_prog *tgt_prog,
 			    u32 btf_id,
 			    struct bpf_attach_target_info *tgt_info);
+int bpf_check_ext_prologue(struct bpf_verifier_log *log,
+			   const struct bpf_prog *tgt_prog, u32 btf_id);
 void bpf_free_kfunc_btf_tab(struct bpf_kfunc_btf_tab *tab);
 
 int mark_chain_precision(struct bpf_verifier_env *env, int regno);
diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index a8739fbd0349..673751b86aee 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -940,6 +940,12 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
 					"Extension writing packet data must take ctx and scalars only\n");
 				return -EINVAL;
 			}
+			if (env->prog->type == BPF_PROG_TYPE_EXT &&
+			    bpf_check_ext_prologue(&env->log,
+						   env->prog->aux->dst_prog,
+						   env->prog->aux->attach_btf_id))
+				return -EINVAL;
+			env->prog->aux->has_prologue = true;
 			new_prog = __bpf_patch_insn_data(env, 0, insn_buf, cnt,
 							 BPF_PATCH_MOVE_TARGET);
 			if (!new_prog)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 63fff641d985..1444be95f577 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -10867,6 +10867,20 @@ static void clear_all_pkt_pointers(struct bpf_verifier_env *env)
 	}));
 }
 
+static bool holds_pkt_pointers(struct bpf_verifier_env *env)
+{
+	struct bpf_func_state *state;
+	struct bpf_reg_state *reg;
+	bool ret = false;
+
+	bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
+		if (reg_is_pkt_pointer_any(reg) || reg_is_dynptr_slice_pkt(reg))
+			ret = true;
+	}));
+
+	return ret;
+}
+
 enum {
 	AT_PKT_END = -1,
 	BEYOND_PKT_END = -2,
@@ -11472,6 +11486,13 @@ static int check_func_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			bpf_diag_mod_begin(env, &caller->regs[BPF_REG_0], NULL, BPF_DIAG_MOD_WRITE);
 		clear_caller_saved_regs(env, caller->regs);
 		invalidate_outgoing_stack_args(env, cur_func(env));
+		/*
+		 * What the callers of a global function hold is not known
+		 * while it is verified.
+		 */
+		if (!env->subprog_info[subprog].changes_pkt_data &&
+		    (state->frame[0]->subprogno || holds_pkt_pointers(env)))
+			sub_aux->called_with_pkt_ptrs = true;
 
 		/*
 		 * A non-void global function returns a 64-bit SCALAR_VALUE in
@@ -22209,6 +22230,41 @@ static int check_unreliable_func_args(struct bpf_verifier_log *log, struct btf *
 	return -EINVAL;
 }
 
+/* Functions may share a type id. The first of them is attached to. */
+static int find_tgt_subprog(const struct bpf_prog *tgt_prog, u32 btf_id)
+{
+	const struct bpf_prog_aux *aux = tgt_prog->aux;
+	int i;
+
+	for (i = 0; i < aux->func_info_cnt; i++)
+		if (aux->func_info[i].type_id == btf_id)
+			return i;
+
+	return -1;
+}
+
+/*
+ * The prologue of an extension gives a cloned skb a new head, unless the
+ * prologue of the target program has done that. The callers of the function
+ * that it replaces must not hold on to the old head.
+ */
+int bpf_check_ext_prologue(struct bpf_verifier_log *log,
+			   const struct bpf_prog *tgt_prog, u32 btf_id)
+{
+	const struct bpf_prog_aux *aux = tgt_prog->aux;
+	int subprog = find_tgt_subprog(tgt_prog, btf_id);
+
+	if (aux->has_prologue)
+		return 0;
+
+	if (subprog >= 0 && !aux->func_info_aux[subprog].called_with_pkt_ptrs)
+		return 0;
+
+	bpf_log(log,
+		"Extension writing packet data cannot replace a function whose callers keep packet pointers\n");
+	return -EINVAL;
+}
+
 /*
  * An extension is verified as a program that runs under the RCU read lock. Does
  * the function that it is to replace?
@@ -22235,7 +22291,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 	char trace_symbol[KSYM_SYMBOL_LEN];
 	const char prefix[] = "btf_trace_";
 	struct bpf_raw_event_map *btp;
-	int ret = 0, subprog = -1, i;
+	int ret = 0, subprog = -1;
 	const struct btf_type *t;
 	bool conservative = true;
 	const char *tname, *fname;
@@ -22274,11 +22330,7 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 			return -EINVAL;
 		}
 
-		for (i = 0; i < aux->func_info_cnt; i++)
-			if (aux->func_info[i].type_id == btf_id) {
-				subprog = i;
-				break;
-			}
+		subprog = find_tgt_subprog(tgt_prog, btf_id);
 		if (subprog == -1) {
 			bpf_log(log, "Subprog %s doesn't exist\n", tname);
 			return -EINVAL;
@@ -22333,6 +22385,9 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 					"Extension program changes packet data, while original does not\n");
 				return -EINVAL;
 			}
+			if (prog->aux->has_prologue &&
+			    bpf_check_ext_prologue(log, tgt_prog, btf_id))
+				return -EINVAL;
 
 			tgt_might_sleep = aux->func
 					  ? aux->func[subprog]->aux->might_sleep
-- 
2.55.0

Re: [PATCH bpf-next v2] bpf: Refuse the prologue for an extension if callers keep packet pointers

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to bpf/bpf-next.git (master)
by Kumar Kartikeya Dwivedi <memxor@gmail.com>:

On Mon, 19 Oct 2026 01:13:27 +0000 you wrote:
> From: Alexei Starovoitov <ast@kernel.org>
> 
> An extension that writes to the packet of a tc, lwt_xmit or sk_skb program
> starts with bpf_unclone_prologue().  If the skb is cloned, that gives it
> a new head.
> 
> It does not count as changing packet data.  changes_pkt_data goes by the
> helpers and kfuncs that a program calls, and whether it gets a prologue
> is known only after it has been compared with its target.  So the extension
> can replace a global function that leaves the packet alone, and the callers
> of that function keep their packet pointers across the call.  These point
> into the old head, which is left to the other clone.  Once that is freed,
> by the reader of a packet socket for example:
> 
> [...]

Here is the summary with links:
  - [bpf-next,v2] bpf: Refuse the prologue for an extension if callers keep packet pointers
    https://git.kernel.org/bpf/bpf-next/c/4f842631f178

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html