Merge branch 'bpf-fixes-for-unwind_goto-and-unwind_exit'

Alexei Starovoitov says:

====================
bpf: Fixes for unwind_goto and unwind_exit

From: Alexei Starovoitov <ast@kernel.org>

Two fixes for what the bots found in v4 [1] of the series that added
unwind_goto and unwind_exit.

Patch 1. unwind_exit of an exception callback is exit for the verifier, but
JITs looked up the return address. The callback returns from the frame of
the main prog. After freplace or a tail call that's a return to a prog, and
the callback could end up in a pad that wasn't verified with it.
It came with 'unwind_exit of an async or an exception callback is exit' in
v4. For an async callback that's fine: it has a frame of its own.

Patch 2. Offloaded progs with the new insns are rejected.

Patch 3. Tests for patch 1. They fail without it.

[1] https://lore.kernel.org/bpf/20261020181320.1284798-1-alexei.starovoitov@gmail.com/
====================

Link: https://patch.msgid.link/20261020231942.2568163-1-alexei.starovoitov@gmail.com
Signed-off-by: Alexei Starovoitov <ast@kernel.org>

arch/arm64/net/bpf_jit_comp.c

@@ -1273,7 +1273,7 @@ static long bpf_unwind_fixup(u64 lr)
 	e = search_bpf_extables(ptrauth_strip_kernel_insn_pac(lr) - 1);
 	/*
 	 * The verifier made sure that unwind_goto follows a call in a prog. When
-	 * the kernel called an async or an exception callback, it's exit.
+	 * the kernel called an async callback, it's exit.
 	 */
 	return e ? e->fixup : 0;
 }

arch/x86/net/bpf_jit_comp.c

@@ -2552,7 +2552,7 @@ static u64 bpf_unwind_target(u64 ret_addr)
 
 	/*
 	 * The verifier made sure that unwind_goto follows a call in a prog. When
-	 * the kernel called an async or an exception callback, it's exit.
+	 * the kernel called an async callback, it's exit.
 	 */
 	return e ? ret_addr + e->fixup : ret_addr;
 }

include/linux/filter.h

@@ -267,9 +267,13 @@ static inline bool bpf_is_unwind_exit(const struct bpf_insn *insn)
 /*
  * The caller of the main prog doesn't tell unwind_exit from exit. An extension
  * is different: its caller is the caller of the subprog that it replaced.
+ * An exception callback returns from the frame of the main prog. After freplace
+ * or a tail call it returns to a prog that wasn't verified with the callback.
  */
 static inline bool bpf_prog_can_unwind(const struct bpf_prog *prog)
 {
+	if (prog->aux->exception_cb)
+		return false;
 	return bpf_is_subprog(prog) || prog->type == BPF_PROG_TYPE_EXT;
 }
 

kernel/bpf/verifier.c

@@ -3462,7 +3462,7 @@ static int check_unwind_insns(struct bpf_verifier_env *env)
 	}
 	if (!seen)
 		return 0;
-	if (!bpf_jit_supports_unwind()) {
+	if (!bpf_jit_supports_unwind() || bpf_prog_is_offloaded(env->prog->aux)) {
 		verbose(env, "JIT does not support unwinding\n");
 		return -ENOTSUPP;
 	}

tools/testing/selftests/bpf/prog_tests/unwind_attach.c

@@ -67,7 +67,7 @@ void test_unwind_attach(void)
 {
 	struct verifier_unwind *tgt;
 	struct bpf_program *prog;
-	int tgt_fd, plain_fd, err;
+	int tgt_fd, plain_fd, shared_fd, tail_fd, throws_fd, err, key = 0;
 
 	if (!is_jit_enabled()) {
 		printf("%s:SKIP:JIT is disabled\n", __func__);
@@ -79,7 +79,10 @@ void test_unwind_attach(void)
 		return;
 	bpf_object__for_each_program(prog, tgt->obj)
 		bpf_program__set_autoload(prog, prog == tgt->progs.calls_global_unwinds ||
-						prog == tgt->progs.calls_global_never_unwinds);
+						prog == tgt->progs.calls_global_never_unwinds ||
+						prog == tgt->progs.shared_pad ||
+						prog == tgt->progs.tail_call_in_subprog ||
+						prog == tgt->progs.exception_callback_unwinds);
 	err = verifier_unwind__load(tgt);
 	if (err == -ENOTSUPP) {
 		printf("%s:SKIP:JIT does not support unwinding\n", __func__);
@@ -107,6 +110,21 @@ void test_unwind_attach(void)
 	attach(plain_fd, "global_never_unwinds", "freplace_unwinds",
 	       "Extension program can unwind, while original does not", 0);
 	ASSERT_EQ(run(tgt_fd), 15, "retval");
+
+	/*
+	 * An exception callback returns from the frame of the main prog. Here
+	 * that's a return to a call that unwind_goto follows. unwind_exit of
+	 * the callback has to be exit: the pad wasn't verified with it.
+	 */
+	shared_fd = bpf_program__fd(tgt->progs.shared_pad);
+	attach(shared_fd, "global_never_unwinds", "freplace_throws", NULL, 3);
+
+	tail_fd = bpf_program__fd(tgt->progs.tail_call_in_subprog);
+	throws_fd = bpf_program__fd(tgt->progs.exception_callback_unwinds);
+	err = bpf_map__update_elem(tgt->maps.prog_array, &key, sizeof(key),
+				   &throws_fd, sizeof(throws_fd), 0);
+	if (ASSERT_OK(err, "prog_array"))
+		ASSERT_EQ(run(tail_fd), 7, "retval");
 out:
 	verifier_unwind__destroy(tgt);
 }

tools/testing/selftests/bpf/progs/unwind_attach.c

@@ -7,6 +7,13 @@
 
 /* The targets are global_unwinds() and global_never_unwinds() in verifier_unwind.c */
 
+#define unwind_exit ".8byte %[insn_unwind_exit];"
+#define __imm_unwind_exit \
+	__imm_insn(insn_unwind_exit, \
+		   BPF_RAW_INSN(BPF_JMP | BPF_EXIT, 0, BPF_UNWIND_EXIT, 0, 0))
+
+extern void bpf_throw(__u64 cookie) __ksym;
+
 int fentry_cnt;
 
 SEC("?fentry/global_unwinds")
@@ -33,11 +40,27 @@ __naked unsigned long freplace_unwinds(void)
 {
 	asm volatile (
 		"r0 = 3;"
-		".8byte %[unwind_exit];"
+		unwind_exit
 		:
-		: __imm_insn(unwind_exit,
-			     BPF_RAW_INSN(BPF_JMP | BPF_EXIT, 0, BPF_UNWIND_EXIT, 0, 0))
+		: __imm_unwind_exit
 	);
 }
 
+/* Not naked: the argument has to have a name in BTF */
+__noinline int throws_cb(__u64 cookie)
+{
+	if (cookie)
+		asm volatile ("r0 = 3;" unwind_exit :: __imm_unwind_exit : "r0");
+	return 0;
+}
+
+/* It has no unwind_exit, so it can replace a subprog that cannot unwind */
+SEC("?freplace/global_never_unwinds")
+__attribute__((btf_decl_tag("exception_callback:throws_cb")))
+unsigned long freplace_throws(void)
+{
+	bpf_throw(1);
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";

tools/testing/selftests/bpf/progs/verifier_unwind.c

@@ -611,6 +611,35 @@ __naked int calls_global_never_unwinds(void)
 	);
 }
 
+/*
+ * The verifier gets to the pad from the first call only, with r6 = 1. The second
+ * unwind_goto stays, since the pad is not dead code.
+ */
+SEC("tc")
+__description("unwind: a pad shared with a call of a subprog that cannot unwind")
+__arch_x86_64
+__arch_arm64
+__load_if_JITed()
+__success __retval(7)
+__naked int shared_pad(void)
+{
+	asm volatile (
+		"r6 = 1;"
+		"call global_returns;"
+		unwind_goto(4)
+		"r6 = 2;"
+		"call global_never_unwinds;"
+		unwind_goto(1)
+		"exit;"
+		"r0 = r6;"
+		"exit;"
+		:
+		: __imm_unwind_goto(4),
+		  __imm_unwind_goto(1)
+		: __clobber_all
+	);
+}
+
 SEC("tc")
 __description("unwind: no unwind_goto after a call of a global subprog")
 __arch_x86_64