bpf: Refuse the prologue for an extension if callers keep packet pointers

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: 7e40781cc8 ("bpf: verifier: Use target program's type for access verifications")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://lore.kernel.org/bpf/20261019011328.1843764-1-alexei.starovoitov@gmail.com
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>

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 {

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);

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)

kernel/bpf/verifier.c

@@ -10878,6 +10878,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,
@@ -11483,6 +11497,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
@@ -22222,6 +22243,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?
@@ -22248,7 +22304,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;
@@ -22287,11 +22343,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;
@@ -22347,6 +22399,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