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, jose.marchesi@oracle.com,
hjl.tools@gmail.com, ndesaulniers@google.com,
samitolvanen@google.com, nathan@kernel.org, ojeda@kernel.org,
kees@kernel.org, alexei.starovoitov@gmail.com,
Andrii Nakryiko <andrii@kernel.org>
Subject: [PATCH 07/14] x86/ibt: Clean up is_endbr()
Date: Fri, 27 Sep 2024 21:49:03 +0200 [thread overview]
Message-ID: <20240927194925.069013308@infradead.org> (raw)
In-Reply-To: <20240927194856.096003183@infradead.org>
Pretty much every caller of is_endbr() actually wants to test something at an
address and ends up doing get_kernel_nofault(). Fold the lot into a more
convenient helper.
Note: this effectively reverts commit a8497506cd2c ("bpf: Avoid
get_kernel_nofault() to fetch kprobe entry IP") which was entirely the
wrong way to go about doing things. The right solution is to optimize
get_kernel_nofault() itself, it really doesn't need STAC/CLAC nor the
speculation barrier. Using __get_user is a historical hack, not a
requirement.
Cc: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/x86/events/core.c | 2 +-
arch/x86/include/asm/ibt.h | 5 +++--
arch/x86/kernel/alternative.c | 19 +++++++++++++------
arch/x86/kernel/kprobes/core.c | 11 +----------
arch/x86/net/bpf_jit_comp.c | 4 ++--
kernel/trace/bpf_trace.c | 14 ++------------
6 files changed, 22 insertions(+), 33 deletions(-)
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -2845,7 +2845,7 @@ static bool is_uprobe_at_func_entry(stru
return true;
/* endbr64 (64-bit only) */
- if (user_64bit_mode(regs) && is_endbr(*(u32 *)auprobe->insn))
+ if (user_64bit_mode(regs) && is_endbr((u32 *)auprobe->insn))
return true;
return false;
--- a/arch/x86/include/asm/ibt.h
+++ b/arch/x86/include/asm/ibt.h
@@ -65,7 +65,7 @@ static __always_inline __attribute_const
return 0x001f0f66; /* osp nopl (%rax) */
}
-static inline bool is_endbr(u32 val)
+static inline bool __is_endbr(u32 val)
{
if (val == gen_endbr_poison())
return true;
@@ -74,6 +74,7 @@ static inline bool is_endbr(u32 val)
return val == gen_endbr();
}
+extern __noendbr bool is_endbr(u32 *val);
extern __noendbr u64 ibt_save(bool disable);
extern __noendbr void ibt_restore(u64 save);
@@ -102,7 +103,7 @@ extern bool __do_kernel_cp_fault(struct
#define __noendbr
-static inline bool is_endbr(u32 val) { return false; }
+static inline bool is_endbr(u32 *val) { return false; }
static inline u64 ibt_save(bool disable) { return 0; }
static inline void ibt_restore(u64 save) { }
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -852,16 +852,23 @@ void __init_or_module noinline apply_ret
#ifdef CONFIG_X86_KERNEL_IBT
+__noendbr bool is_endbr(u32 *val)
+{
+ u32 endbr;
+
+ if (get_kernel_nofault(endbr, val))
+ return false;
+
+ return __is_endbr(endbr);
+}
+
static void poison_cfi(void *addr);
static void __init_or_module poison_endbr(void *addr, bool warn)
{
- u32 endbr, poison = gen_endbr_poison();
-
- if (WARN_ON_ONCE(get_kernel_nofault(endbr, addr)))
- return;
+ u32 poison = gen_endbr_poison();
- if (!is_endbr(endbr)) {
+ if (!is_endbr(addr)) {
WARN_ON_ONCE(warn);
return;
}
@@ -988,7 +995,7 @@ static u32 cfi_seed __ro_after_init;
static u32 cfi_rehash(u32 hash)
{
hash ^= cfi_seed;
- while (unlikely(is_endbr(hash) || is_endbr(-hash))) {
+ while (unlikely(__is_endbr(hash) || __is_endbr(-hash))) {
bool lsb = hash & 1;
hash >>= 1;
if (lsb)
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -373,16 +373,7 @@ static bool can_probe(unsigned long padd
kprobe_opcode_t *arch_adjust_kprobe_addr(unsigned long addr, unsigned long offset,
bool *on_func_entry)
{
- u32 insn;
-
- /*
- * Since 'addr' is not guaranteed to be safe to access, use
- * copy_from_kernel_nofault() to read the instruction:
- */
- if (copy_from_kernel_nofault(&insn, (void *)addr, sizeof(u32)))
- return NULL;
-
- if (is_endbr(insn)) {
+ if (is_endbr((u32 *)addr)) {
*on_func_entry = !offset || offset == 4;
if (*on_func_entry)
offset = 4;
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -625,7 +625,7 @@ int bpf_arch_text_poke(void *ip, enum bp
* See emit_prologue(), for IBT builds the trampoline hook is preceded
* with an ENDBR instruction.
*/
- if (is_endbr(*(u32 *)ip))
+ if (is_endbr(ip))
ip += ENDBR_INSN_SIZE;
return __bpf_arch_text_poke(ip, t, old_addr, new_addr);
@@ -2971,7 +2971,7 @@ static int __arch_prepare_bpf_trampoline
/* skip patched call instruction and point orig_call to actual
* body of the kernel function.
*/
- if (is_endbr(*(u32 *)orig_call))
+ if (is_endbr(orig_call))
orig_call += ENDBR_INSN_SIZE;
orig_call += X86_PATCH_SIZE;
}
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1027,19 +1027,9 @@ static const struct bpf_func_proto bpf_g
#ifdef CONFIG_X86_KERNEL_IBT
static unsigned long get_entry_ip(unsigned long fentry_ip)
{
- u32 instr;
+ if (is_endbr((void *)(fentry_ip - ENDBR_INSN_SIZE)))
+ return fentry_ip - ENDBR_INSN_SIZE;
- /* We want to be extra safe in case entry ip is on the page edge,
- * but otherwise we need to avoid get_kernel_nofault()'s overhead.
- */
- if ((fentry_ip & ~PAGE_MASK) < ENDBR_INSN_SIZE) {
- if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
- return fentry_ip;
- } else {
- instr = *(u32 *)(fentry_ip - ENDBR_INSN_SIZE);
- }
- if (is_endbr(instr))
- fentry_ip -= ENDBR_INSN_SIZE;
return fentry_ip;
}
#else
next prev parent reply other threads:[~2024-09-27 19:51 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-09-27 19:48 [PATCH 00/14] x86/ibt: FineIBT-BHI Peter Zijlstra
2024-09-27 19:48 ` [PATCH 01/14] x86/cfi: Wreck things Peter Zijlstra
2024-09-27 23:15 ` Josh Poimboeuf
2024-09-28 13:31 ` Peter Zijlstra
2024-09-30 21:42 ` Josh Poimboeuf
2024-09-27 19:48 ` [PATCH 02/14] x86/boot: Mark start_secondary() with __noendbr Peter Zijlstra
2024-09-27 19:48 ` [PATCH 03/14] x86/alternative: Simplify callthunk patching Peter Zijlstra
2024-09-27 23:27 ` Josh Poimboeuf
2024-09-27 19:49 ` [PATCH 04/14] objtool/x86: Add .tail_call_sites Peter Zijlstra
2024-09-27 23:42 ` Josh Poimboeuf
2024-10-09 15:25 ` Peter Zijlstra
2024-10-10 4:55 ` Josh Poimboeuf
2024-09-27 19:49 ` [PATCH 05/14] objtool: Rename the skylake hack to --direct-call Peter Zijlstra
2024-09-27 19:49 ` [PATCH 06/14] x86/traps: Prepare for ENDBR poison UD1 usage Peter Zijlstra
2024-09-27 19:49 ` Peter Zijlstra [this message]
2024-09-28 0:04 ` [PATCH 07/14] x86/ibt: Clean up is_endbr() Josh Poimboeuf
2024-09-28 13:08 ` Peter Zijlstra
2024-09-29 17:32 ` Alexei Starovoitov
2024-09-30 8:30 ` Peter Zijlstra
2024-09-30 9:33 ` Peter Zijlstra
2024-09-30 16:43 ` Alexei Starovoitov
2024-09-30 20:58 ` Andrii Nakryiko
2024-09-27 19:49 ` [PATCH 08/14] x86/ibt: Clean up poison_endbr() Peter Zijlstra
2024-09-27 19:49 ` [PATCH 09/14] x86/ibt: Implement IBT+ Peter Zijlstra
2024-09-28 1:07 ` Josh Poimboeuf
2024-09-28 13:12 ` Peter Zijlstra
2024-09-29 17:38 ` Alexei Starovoitov
2024-09-30 8:23 ` Peter Zijlstra
2024-09-30 17:00 ` Alexei Starovoitov
2024-11-05 10:40 ` Peter Zijlstra
2024-09-27 19:49 ` [PATCH 10/14] x86/early_printk: Harden early_serial Peter Zijlstra
2024-09-27 19:49 ` [PATCH 11/14] llvm: kCFI pointer stuff Peter Zijlstra
2024-09-29 17:53 ` Alexei Starovoitov
2024-09-30 8:27 ` Peter Zijlstra
2024-09-30 16:59 ` Alexei Starovoitov
2024-10-01 10:21 ` Peter Zijlstra
2024-10-02 16:48 ` Alexei Starovoitov
2024-10-30 6:29 ` Constable, Scott D
2024-10-30 20:07 ` Constable, Scott D
2024-09-27 19:49 ` [PATCH 12/14] x86: Hacks for hacked up llvm Peter Zijlstra
2024-09-27 19:49 ` [PATCH 13/14] x86: BHI stubs Peter Zijlstra
2024-09-28 1:37 ` Josh Poimboeuf
2024-09-28 13:23 ` Peter Zijlstra
2024-09-30 21:30 ` Josh Poimboeuf
2024-09-30 21:46 ` Josh Poimboeuf
2024-09-30 22:23 ` Andrew Cooper
2024-09-30 22:38 ` Josh Poimboeuf
2024-09-30 22:52 ` Andrew Cooper
2024-10-01 11:03 ` Peter Zijlstra
2024-10-01 11:20 ` Andrew Cooper
2024-10-03 12:17 ` Peter Zijlstra
2024-10-03 13:59 ` Andrew Cooper
2024-10-14 17:50 ` Constable, Scott D
2024-10-14 21:54 ` Andrew Cooper
2024-10-21 15:06 ` Constable, Scott D
2024-10-29 5:59 ` Joao Moreira
2024-09-27 19:49 ` [PATCH 14/14] x86/fineibt: Add FineIBT+BHI mitigation Peter Zijlstra
2024-09-28 1:50 ` Josh Poimboeuf
2024-09-28 13:16 ` Peter Zijlstra
2024-10-28 5:45 ` Constable, Scott D
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=20240927194925.069013308@infradead.org \
--to=peterz@infradead.org \
--cc=alexei.starovoitov@gmail.com \
--cc=alyssa.milburn@intel.com \
--cc=andrew.cooper3@citrix.com \
--cc=andrii@kernel.org \
--cc=hjl.tools@gmail.com \
--cc=joao@overdrivepizza.com \
--cc=jose.marchesi@oracle.com \
--cc=jpoimboe@kernel.org \
--cc=kees@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=ojeda@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®