diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c index b3efe0668e86..326e9e1dea0a 100644 --- a/arch/arm64/net/bpf_jit_comp.c +++ b/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; } diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index d3ab36281144..6a2fe3d0039d 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/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; } diff --git a/include/linux/filter.h b/include/linux/filter.h index fccd8db33647..e0218e677a1d 100644 --- a/include/linux/filter.h +++ b/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; } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 7d7193b087f6..5601d2056218 100644 --- a/kernel/bpf/verifier.c +++ b/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; } diff --git a/tools/testing/selftests/bpf/prog_tests/unwind_attach.c b/tools/testing/selftests/bpf/prog_tests/unwind_attach.c index ada8b3273ccb..37e03b3e0ee8 100644 --- a/tools/testing/selftests/bpf/prog_tests/unwind_attach.c +++ b/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); } diff --git a/tools/testing/selftests/bpf/progs/unwind_attach.c b/tools/testing/selftests/bpf/progs/unwind_attach.c index df73c26c1ff4..9aff7c6bda23 100644 --- a/tools/testing/selftests/bpf/progs/unwind_attach.c +++ b/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"; diff --git a/tools/testing/selftests/bpf/progs/verifier_unwind.c b/tools/testing/selftests/bpf/progs/verifier_unwind.c index 0a75cb8af67f..5a21345c34d4 100644 --- a/tools/testing/selftests/bpf/progs/verifier_unwind.c +++ b/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