bpf: Refuse the prologue for an extension that cannot take it

A tc, lwt_xmit or sk_skb program that writes to packet data starts with
bpf_unclone_prologue().  It takes the skb from R1, and if the skb is cloned
it calls bpf_skb_pull_data() and restores R1 only.

That suits the main function of a program.  An extension gets the prologue
too, but it starts with the arguments of the function that it replaces.
If the first one is not the context, something else is taken for an skb:

  BUG: KASAN: stack-out-of-bounds in skb_ensure_writable+0x84/0x1a0
  Read of size 8 at addr ffffc90000270ad0 by task uncl/105
   skb_ensure_writable+0x84/0x1a0
   bpf_skb_pull_data+0x32/0xf0
   bpf_prog_853eb1991d35e093_ext_slice+0x4f/0x321
   bpf_prog_9eb18752b5d0959a_t_glob+0x2ac/0x2de
  BUG: unable to handle page fault for address: 0000000082298821

And whatever R2-R5 held is lost, while the verifier goes on trusting
pointers in them.

Refuse an extension that needs a prologue unless it gets the context as
its first argument and scalars beside it.  A scalar is still clobbered if
the skb is cloned, which the verifier can cope with.  Extensions of XDP,
sk_msg and sockopt programs get no prologue and load as before.

Fixes: be8704ff07d2 ("bpf: Introduce dynamic program extensions")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261017230950.802052-12-alexei.starovoitov@gmail.com

1 files changed, 25 insertions, 0 deletions

diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c
index fe59e3114d82..a8739fbd0349 100644
--- a/kernel/bpf/fixups.c
+++ b/kernel/bpf/fixups.c
@@ -858,6 +858,25 @@ apply_patch_buffer:
 	return 0;
 }
 
+/*
+ * The prologue takes the context from R1, and it can clobber R2-R5. An
+ * extension gets the arguments of the function that it replaces.
+ */
+static bool ext_prog_takes_prologue(const struct bpf_verifier_env *env)
+{
+	const struct bpf_subprog_info *sub = &env->subprog_info[0];
+	int i, cnt = min_t(int, sub->arg_slot_cnt, MAX_BPF_FUNC_REG_ARGS);
+
+	if (!cnt || sub->args[0].arg_type != ARG_PTR_TO_CTX)
+		return false;
+
+	for (i = 1; i < cnt; i++)
+		if (sub->args[i].arg_type != ARG_SCALAR)
+			return false;
+
+	return true;
+}
+
 /* convert load instructions that access fields of a context type into a
  * sequence of instructions that access fields of the underlying structure:
  *     struct __sk_buff    -> struct sk_buff
@@ -915,6 +934,12 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env)
 			verifier_bug(env, "prologue is too long");
 			return -EFAULT;
 		} else if (cnt) {
+			if (env->prog->type == BPF_PROG_TYPE_EXT &&
+			    !ext_prog_takes_prologue(env)) {
+				verbose(env,
+					"Extension writing packet data must take ctx and scalars only\n");
+				return -EINVAL;
+			}
 			new_prog = __bpf_patch_insn_data(env, 0, insn_buf, cnt,
 							 BPF_PATCH_MOVE_TARGET);
 			if (!new_prog)