From: Peter Zijlstra <peterz@infradead.org>
To: x86@kernel.org
Cc: linux-kernel@vger.kernel.org, peterz@infradead.org,
alyssa.milburn@intel.com, scott.d.constable@intel.com,
joao@overdrivepizza.com, andrew.cooper3@citrix.com,
jpoimboe@kernel.org, alexei.starovoitov@gmail.com,
ebiggers@kernel.org, samitolvanen@google.com, kees@kernel.org
Subject: [PATCH 7/8] x86/ibt: Clean up poison_endbr()
Date: Tue, 05 Nov 2024 12:39:08 +0100 [thread overview]
Message-ID: <20241105114522.514586258@infradead.org> (raw)
In-Reply-To: 20241105113901.348320374@infradead.org
Basically, get rid of the .warn argument and explicitly don't call the
function when we know there isn't an endbr. This makes the calling
code clearer.
Note: perhaps don't add functions to .cfi_sites when the function
doesn't have endbr -- OTOH why would the compiler emit the prefix if
it has already determined there are no indirect callers and has
omitted the ENDBR instruction.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/kernel/alternative.c | 42 +++++++++++++++++++++++++++++++++++-------
1 file changed, 35 insertions(+), 7 deletions(-)
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -864,14 +864,12 @@ __noendbr bool is_endbr(u32 *val)
static void poison_cfi(void *addr);
-static void __init_or_module poison_endbr(void *addr, bool warn)
+static void __init_or_module poison_endbr(void *addr)
{
u32 poison = gen_endbr_poison();
- if (!is_endbr(addr)) {
- WARN_ON_ONCE(warn);
+ if (WARN_ON_ONCE(!is_endbr(addr)))
return;
- }
DPRINTK(ENDBR, "ENDBR at: %pS (%px)", addr, addr);
@@ -896,7 +894,7 @@ void __init_or_module noinline apply_sea
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- poison_endbr(addr, true);
+ poison_endbr(addr);
if (IS_ENABLED(CONFIG_FINEIBT))
poison_cfi(addr - 16);
}
@@ -1203,6 +1201,14 @@ static int cfi_rewrite_preamble(s32 *sta
void *addr = (void *)s + *s;
u32 hash;
+ /*
+ * When the function doesn't start with ENDBR the compiler will
+ * have determined there are no indirect calls to it and we
+ * don't need no CFI either.
+ */
+ if (!is_endbr(addr + 16))
+ continue;
+
hash = decode_preamble_hash(addr);
if (WARN(!hash, "no CFI hash found at: %pS %px %*ph\n",
addr, addr, 5, addr))
@@ -1223,7 +1229,10 @@ static void cfi_rewrite_endbr(s32 *start
for (s = start; s < end; s++) {
void *addr = (void *)s + *s;
- poison_endbr(addr+16, false);
+ if (!is_endbr(addr + 16))
+ continue;
+
+ poison_endbr(addr + 16);
}
}
@@ -1356,9 +1365,23 @@ static inline void poison_hash(void *add
static void poison_cfi(void *addr)
{
+ /*
+ * Compilers manage to be inconsistent with ENDBR vs __cfi prefixes,
+ * some (static) functions for which they can determine the address
+ * is never taken do not get a __cfi prefix, but *DO* get an ENDBR.
+ *
+ * As such, these functions will get sealed, but we need to be careful
+ * to not unconditionally scribble the previous function.
+ */
switch (cfi_mode) {
case CFI_FINEIBT:
/*
+ * FineIBT prefix should start with an ENDBR.
+ */
+ if (!is_endbr(addr))
+ break;
+
+ /*
* __cfi_\func:
* osp nopl (%rax)
* subl $0, %r10d
@@ -1366,12 +1389,17 @@ static void poison_cfi(void *addr)
* ud2
* 1: nop
*/
- poison_endbr(addr, false);
+ poison_endbr(addr);
poison_hash(addr + fineibt_preamble_hash);
break;
case CFI_KCFI:
/*
+ * kCFI prefix should start with a valid hash.
+ */
+ if (!decode_preamble_hash(addr))
+ break;
+ /*
* __cfi_\func:
* movl $0, %eax
* .skip 11, 0x90
next prev parent reply other threads:[~2024-11-05 11:47 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-11-05 11:39 [PATCH 0/8] x86: kCFI and IBT cleanups Peter Zijlstra
2024-11-05 11:39 ` [PATCH 1/8] x86,kcfi: Fix EXPORT_SYMBOL vs kCFI Peter Zijlstra
2024-11-05 14:16 ` Christoph Hellwig
2024-11-05 14:27 ` Peter Zijlstra
2024-11-05 14:32 ` Christoph Hellwig
2024-11-05 14:58 ` Peter Zijlstra
2024-11-05 15:41 ` Christoph Hellwig
2024-11-05 15:47 ` Peter Zijlstra
2024-11-05 16:11 ` Peter Zijlstra
2024-11-09 9:14 ` David Laight
2024-11-05 11:39 ` [PATCH 2/8] x86/cfi: Clean up linkage Peter Zijlstra
2024-11-05 11:39 ` [PATCH 3/8] x86/boot: Mark start_secondary() with __noendbr Peter Zijlstra
2024-11-05 11:39 ` [PATCH 4/8] x86/alternative: Simplify callthunk patching Peter Zijlstra
2024-11-05 11:39 ` [PATCH 5/8] x86/traps: Cleanup and robustify decode_bug() Peter Zijlstra
2024-11-05 15:19 ` Andrew Cooper
2024-11-05 11:39 ` [PATCH 6/8] x86/ibt: Clean up is_endbr() Peter Zijlstra
2024-11-05 11:39 ` Peter Zijlstra [this message]
2024-11-05 11:39 ` [PATCH 8/8] x86/early_printk: Harden early_serial Peter Zijlstra
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=20241105114522.514586258@infradead.org \
--to=peterz@infradead.org \
--cc=alexei.starovoitov@gmail.com \
--cc=alyssa.milburn@intel.com \
--cc=andrew.cooper3@citrix.com \
--cc=ebiggers@kernel.org \
--cc=joao@overdrivepizza.com \
--cc=jpoimboe@kernel.org \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=samitolvanen@google.com \
--cc=scott.d.constable@intel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.