From: Peter Zijlstra <peterz@infradead.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
Mark Rutland <mark.rutland@arm.com>, X86 ML <x86@kernel.org>,
Kees Cook <keescook@chromium.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
Sedat Dilek <sedat.dilek@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>,
linux-hardening@vger.kernel.org,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
llvm@lists.linux.dev
Subject: [PATCH] static_call,x86: Robustify trampoline patching
Date: Sat, 30 Oct 2021 09:47:58 +0200 [thread overview]
Message-ID: <20211030074758.GT174703@worktop.programming.kicks-ass.net> (raw)
In-Reply-To: <20211029200324.GR174703@worktop.programming.kicks-ass.net>
On Fri, Oct 29, 2021 at 10:03:24PM +0200, Peter Zijlstra wrote:
> So I had a bit of a peek at what clang generates:
>
> 3fa4: 48 c7 c7 00 00 00 00 mov $0x0,%rdi 3fa7: R_X86_64_32S __SCK__x86_pmu_handle_irq
> 3fab: 48 c7 c6 00 00 00 00 mov $0x0,%rsi 3fae: R_X86_64_32S __SCT__x86_pmu_handle_irq.cfi_jt
> 3fb2: e8 00 00 00 00 call 3fb7 <init_hw_perf_events+0x1dc> 3fb3: R_X86_64_PLT32 __static_call_update-0x4
>
> So this then gives the trampoline jump table entry to
> __static_call_update(), with the result that it will rewrite the
> jump-table entry, not the trampoline!
>
> Now it so happens that the trampoline looks *exactly* like the
> jump-table entry (one jmp.d32 instruction), so in that regards it'll
> again 'work'.
>
> But this is all really, as in *really*, wrong. And I'm really sad I'm
> the one to have to discover this, even though I've mentioned
> static_call()s being tricky in previous reviews.
The below makes the clang-cfi build properly sick:
[ 0.000000] trampoline signature fail
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] kernel BUG at arch/x86/kernel/static_call.c:65!
---
Subject: static_call,x86: Robustify trampoline patching
Add a few signature bytes after the static call trampoline and verify
those bytes match before patching the trampoline. This avoids patching
random other JMPs (such as CFI jump-table entries) instead.
These bytes decode as:
d: 53 push %rbx
e: 43 54 rex.XB push %r12
And happen to spell "SCT".
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/include/asm/static_call.h | 1 +
arch/x86/kernel/static_call.c | 14 ++++++++++----
tools/objtool/check.c | 3 +++
3 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/static_call.h b/arch/x86/include/asm/static_call.h
index cbb67b6030f9..39ebe0511869 100644
--- a/arch/x86/include/asm/static_call.h
+++ b/arch/x86/include/asm/static_call.h
@@ -27,6 +27,7 @@
".globl " STATIC_CALL_TRAMP_STR(name) " \n" \
STATIC_CALL_TRAMP_STR(name) ": \n" \
insns " \n" \
+ ".byte 0x53, 0x43, 0x54 \n" \
".type " STATIC_CALL_TRAMP_STR(name) ", @function \n" \
".size " STATIC_CALL_TRAMP_STR(name) ", . - " STATIC_CALL_TRAMP_STR(name) " \n" \
".popsection \n")
diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
index ea028e736831..9c407a33a774 100644
--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -56,10 +56,15 @@ static void __ref __static_call_transform(void *insn, enum insn_type type, void
text_poke_bp(insn, code, size, emulate);
}
-static void __static_call_validate(void *insn, bool tail)
+static void __static_call_validate(void *insn, bool tail, bool tramp)
{
u8 opcode = *(u8 *)insn;
+ if (tramp && memcmp(insn+5, "SCT", 3)) {
+ pr_err("trampoline signature fail");
+ BUG();
+ }
+
if (tail) {
if (opcode == JMP32_INSN_OPCODE ||
opcode == RET_INSN_OPCODE)
@@ -74,7 +79,8 @@ static void __static_call_validate(void *insn, bool tail)
/*
* If we ever trigger this, our text is corrupt, we'll probably not live long.
*/
- WARN_ONCE(1, "unexpected static_call insn opcode 0x%x at %pS\n", opcode, insn);
+ pr_err("unexpected static_call insn opcode 0x%x at %pS\n", opcode, insn);
+ BUG();
}
static inline enum insn_type __sc_insn(bool null, bool tail)
@@ -97,12 +103,12 @@ void arch_static_call_transform(void *site, void *tramp, void *func, bool tail)
mutex_lock(&text_mutex);
if (tramp) {
- __static_call_validate(tramp, true);
+ __static_call_validate(tramp, true, true);
__static_call_transform(tramp, __sc_insn(!func, true), func);
}
if (IS_ENABLED(CONFIG_HAVE_STATIC_CALL_INLINE) && site) {
- __static_call_validate(site, tail);
+ __static_call_validate(site, tail, false);
__static_call_transform(site, __sc_insn(!func, tail), func);
}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index fb3f251ea021..6d5951113d53 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -3310,6 +3310,9 @@ static bool ignore_unreachable_insn(struct objtool_file *file, struct instructio
if (!insn->func)
return false;
+ if (insn->func->static_call_tramp)
+ return true;
+
/*
* CONFIG_UBSAN_TRAP inserts a UD2 when it sees
* __builtin_unreachable(). The BUG() macro has an unreachable() after
next prev parent reply other threads:[~2021-10-30 7:50 UTC|newest]
Thread overview: 117+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-13 18:16 [PATCH v5 00/15] x86: Add support for Clang CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 01/15] objtool: Add CONFIG_CFI_CLANG support Sami Tolvanen
2021-10-13 18:59 ` Kees Cook
2021-10-14 0:44 ` Josh Poimboeuf
2021-10-14 10:22 ` Peter Zijlstra
2021-10-14 19:20 ` Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 02/15] objtool: Add ASM_STACK_FRAME_NON_STANDARD Sami Tolvanen
2021-10-13 18:59 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 03/15] linkage: Add DECLARE_NOT_CALLED_FROM_C Sami Tolvanen
2021-10-13 19:00 ` Kees Cook
2021-10-15 2:51 ` Andy Lutomirski
2021-10-15 15:35 ` Sami Tolvanen
2021-10-15 15:55 ` Thomas Gleixner
2021-10-15 16:22 ` Andy Lutomirski
2021-10-15 16:47 ` Sami Tolvanen
2021-10-15 17:34 ` Andy Lutomirski
2021-10-15 17:57 ` Thomas Gleixner
2021-10-15 18:42 ` Sami Tolvanen
2021-10-15 19:35 ` Andy Lutomirski
2021-10-15 20:37 ` Sami Tolvanen
2021-10-16 21:12 ` Josh Poimboeuf
2021-10-18 17:08 ` Sami Tolvanen
2021-10-15 22:17 ` Thomas Gleixner
2021-10-16 21:16 ` Josh Poimboeuf
2021-10-13 18:16 ` [PATCH v5 04/15] cfi: Add DEFINE_CFI_IMMEDIATE_RETURN_STUB Sami Tolvanen
2021-10-13 19:02 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 05/15] tracepoint: Exclude tp_stub_func from CFI checking Sami Tolvanen
2021-10-13 19:03 ` Kees Cook
2021-10-13 19:20 ` Steven Rostedt
2021-10-13 18:16 ` [PATCH v5 06/15] ftrace: Use an opaque type for functions not callable from C Sami Tolvanen
2021-10-13 19:04 ` Kees Cook
2021-10-13 19:20 ` Steven Rostedt
2021-10-13 18:16 ` [PATCH v5 07/15] lkdtm: Disable UNSET_SMEP with CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 08/15] lkdtm: Use an opaque type for lkdtm_rodata_do_nothing Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 09/15] x86: Use an opaque type for functions not callable from C Sami Tolvanen
2021-10-14 11:21 ` Borislav Petkov
2021-10-14 16:07 ` Kees Cook
2021-10-14 17:31 ` Borislav Petkov
2021-10-14 18:24 ` Sami Tolvanen
2021-10-14 19:00 ` Nick Desaulniers
2021-10-14 18:47 ` Kees Cook
2021-10-14 18:52 ` Steven Rostedt
2021-10-14 19:06 ` Josh Poimboeuf
2021-10-13 18:16 ` [PATCH v5 10/15] x86/purgatory: Disable CFI Sami Tolvanen
2021-10-13 19:05 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 11/15] x86, relocs: Ignore __typeid__ relocations Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 12/15] x86, module: " Sami Tolvanen
2021-10-13 18:55 ` Kees Cook
2021-10-13 18:16 ` [PATCH v5 13/15] x86, cpu: Use LTO for cpu.c with CFI Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 14/15] x86, kprobes: Fix optprobe_template_func type mismatch Sami Tolvanen
2021-10-13 18:16 ` [PATCH v5 15/15] x86, build: Allow CONFIG_CFI_CLANG to be selected Sami Tolvanen
2021-10-13 18:56 ` Kees Cook
2021-10-13 19:07 ` [PATCH v5 00/15] x86: Add support for Clang CFI Kees Cook
2021-10-19 10:06 ` Alexander Lobakin
2021-10-19 15:40 ` Sami Tolvanen
2021-10-21 10:27 ` Alexander Lobakin
2021-10-26 20:16 ` Peter Zijlstra
2021-10-27 10:02 ` David Laight
2021-10-27 10:17 ` Peter Zijlstra
2021-10-27 12:05 ` Mark Rutland
2021-10-27 12:22 ` Ard Biesheuvel
2021-10-27 12:48 ` Peter Zijlstra
2021-10-27 13:04 ` Peter Zijlstra
2021-10-27 13:30 ` Ard Biesheuvel
2021-10-27 14:03 ` Peter Zijlstra
2021-10-27 14:18 ` Ard Biesheuvel
2021-10-27 14:36 ` Peter Zijlstra
2021-10-27 15:50 ` Sami Tolvanen
2021-10-27 15:55 ` Ard Biesheuvel
2021-10-29 20:03 ` Peter Zijlstra
2021-10-30 7:47 ` Peter Zijlstra [this message]
2021-10-30 8:16 ` [PATCH] static_call,x86: Robustify trampoline patching Peter Zijlstra
2021-11-02 17:35 ` Kees Cook
2021-11-02 18:15 ` Peter Zijlstra
2021-11-15 13:09 ` Rasmus Villemoes
2021-10-30 17:19 ` Ard Biesheuvel
2021-10-30 18:02 ` Peter Zijlstra
2021-10-30 18:55 ` Ard Biesheuvel
2021-10-31 16:24 ` Ard Biesheuvel
2021-10-31 16:39 ` Peter Zijlstra
2021-10-31 16:44 ` Ard Biesheuvel
2021-10-31 20:09 ` Peter Zijlstra
2021-10-31 20:21 ` Ard Biesheuvel
2021-10-31 20:44 ` Peter Zijlstra
2021-10-31 23:36 ` Ard Biesheuvel
2021-11-01 9:01 ` Peter Zijlstra
2021-11-01 9:36 ` David Laight
2021-11-01 14:14 ` Ard Biesheuvel
2021-11-02 12:57 ` Peter Zijlstra
2021-11-02 15:15 ` Peter Zijlstra
2021-11-02 17:44 ` Ard Biesheuvel
2021-11-02 18:14 ` Peter Zijlstra
2021-11-02 18:17 ` Peter Zijlstra
2021-11-02 18:18 ` Ard Biesheuvel
2021-11-02 21:48 ` Peter Zijlstra
2021-11-02 18:10 ` Kees Cook
2021-11-02 21:02 ` Andy Lutomirski
2021-11-02 23:13 ` Kees Cook
2021-11-03 0:20 ` Andy Lutomirski
2021-11-03 8:35 ` Peter Zijlstra
2021-11-03 10:01 ` David Laight
2021-11-03 19:32 ` Andy Lutomirski
2021-11-02 21:19 ` Peter Zijlstra
2021-11-11 12:15 ` [tip: locking/urgent] " tip-bot2 for Peter Zijlstra
2021-10-30 19:07 ` [PATCH v5 00/15] x86: Add support for Clang CFI Sami Tolvanen
2021-10-27 17:11 ` Kees Cook
2021-10-27 21:21 ` Peter Zijlstra
2021-10-27 22:27 ` Kees Cook
2021-10-28 11:09 ` Peter Zijlstra
2021-10-28 17:12 ` Kees Cook
2021-10-28 20:29 ` Peter Zijlstra
2021-11-02 17:26 ` Kees Cook
2021-11-01 4:13 ` Andy Lutomirski
2021-10-27 12:46 ` Peter Zijlstra
2021-10-27 12:55 ` David Laight
2021-10-27 13:17 ` Mark Rutland
2021-10-27 21:31 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20211030074758.GT174703@worktop.programming.kicks-ass.net \
--to=peterz@infradead.org \
--cc=ardb@kernel.org \
--cc=jpoimboe@redhat.com \
--cc=keescook@chromium.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=llvm@lists.linux.dev \
--cc=mark.rutland@arm.com \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=rostedt@goodmis.org \
--cc=samitolvanen@google.com \
--cc=sedat.dilek@gmail.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).