All of lore.kernel.org
 help / color / mirror / Atom feed
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 5/8] x86/traps: Cleanup and robustify decode_bug()
Date: Tue, 05 Nov 2024 12:39:06 +0100	[thread overview]
Message-ID: <20241105114522.285032152@infradead.org> (raw)
In-Reply-To: 20241105113901.348320374@infradead.org

Notably, don't attempt to decode an immediate when MOD == 3.

Additionally have it return the instruction length, such that WARN
like bugs can more reliably skip to the correct instruction.

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/bug.h |    5 +-
 arch/x86/include/asm/ibt.h |    4 +-
 arch/x86/kernel/cet.c      |   18 +++++++---
 arch/x86/kernel/traps.c    |   77 ++++++++++++++++++++++++++++++++-------------
 4 files changed, 73 insertions(+), 31 deletions(-)

--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -22,8 +22,9 @@
 #define SECOND_BYTE_OPCODE_UD2	0x0b
 
 #define BUG_NONE		0xffff
-#define BUG_UD1			0xfffe
-#define BUG_UD2			0xfffd
+#define BUG_UD2			0xfffe
+#define BUG_UD1			0xfffd
+#define BUG_UD1_UBSAN		0xfffc
 
 #ifdef CONFIG_GENERIC_BUG
 
--- a/arch/x86/include/asm/ibt.h
+++ b/arch/x86/include/asm/ibt.h
@@ -41,7 +41,7 @@
 	_ASM_PTR fname "\n\t"				\
 	".popsection\n\t"
 
-static inline __attribute_const__ u32 gen_endbr(void)
+static __always_inline __attribute_const__ u32 gen_endbr(void)
 {
 	u32 endbr;
 
@@ -56,7 +56,7 @@ static inline __attribute_const__ u32 ge
 	return endbr;
 }
 
-static inline __attribute_const__ u32 gen_endbr_poison(void)
+static __always_inline __attribute_const__ u32 gen_endbr_poison(void)
 {
 	/*
 	 * 4 byte NOP that isn't NOP4 (in fact it is OSP NOP3), such that it
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -94,10 +94,17 @@ __always_inline int is_valid_bugaddr(uns
 
 /*
  * Check for UD1 or UD2, accounting for Address Size Override Prefixes.
- * If it's a UD1, get the ModRM byte to pass along to UBSan.
+ * If it's a UD1, further decode to determine its use:
+ *
+ * UBSan{0}:     67 0f b9 00             ud1    (%eax),%eax
+ * UBSan{10}:    67 0f b9 40 10          ud1    0x10(%eax),%eax
+ * static_call:  0f b9 cc                ud1    %esp,%ecx
+ *
+ * Notably UBSAN uses EAX, static_call uses ECX.
  */
-__always_inline int decode_bug(unsigned long addr, u32 *imm)
+__always_inline int decode_bug(unsigned long addr, s32 *imm, int *len)
 {
+	unsigned long start = addr;
 	u8 v;
 
 	if (addr < TASK_SIZE_MAX)
@@ -110,24 +117,37 @@ __always_inline int decode_bug(unsigned
 		return BUG_NONE;
 
 	v = *(u8 *)(addr++);
-	if (v == SECOND_BYTE_OPCODE_UD2)
+	if (v == SECOND_BYTE_OPCODE_UD2) {
+		*len = addr - start;
 		return BUG_UD2;
+	}
 
-	if (!IS_ENABLED(CONFIG_UBSAN_TRAP) || v != SECOND_BYTE_OPCODE_UD1)
+	if (v != SECOND_BYTE_OPCODE_UD1)
 		return BUG_NONE;
 
-	/* Retrieve the immediate (type value) for the UBSAN UD1 */
-	v = *(u8 *)(addr++);
-	if (X86_MODRM_RM(v) == 4)
-		addr++;
-
 	*imm = 0;
-	if (X86_MODRM_MOD(v) == 1)
-		*imm = *(u8 *)addr;
-	else if (X86_MODRM_MOD(v) == 2)
-		*imm = *(u32 *)addr;
-	else
-		WARN_ONCE(1, "Unexpected MODRM_MOD: %u\n", X86_MODRM_MOD(v));
+	v = *(u8 *)(addr++);		/* ModRM */
+
+	/* Decode immediate, if present */
+	if (X86_MODRM_MOD(v) != 3) {
+		if (X86_MODRM_RM(v) == 4)
+			addr++;		/* Skip SIB byte */
+
+		if (X86_MODRM_MOD(v) == 1) {
+			*imm = *(s8 *)addr;
+			addr += 1;
+
+		} else if (X86_MODRM_MOD(v) == 2) {
+			*imm = *(s32 *)addr;
+			addr += 4;
+		}
+	}
+
+	/* record instruction length */
+	*len = addr - start;
+
+	if (X86_MODRM_REG(v) == 0)	/* EAX */
+		return BUG_UD1_UBSAN;
 
 	return BUG_UD1;
 }
@@ -258,10 +278,10 @@ static inline void handle_invalid_op(str
 static noinstr bool handle_bug(struct pt_regs *regs)
 {
 	bool handled = false;
-	int ud_type;
-	u32 imm;
+	int ud_type, ud_len;
+	s32 ud_imm;
 
-	ud_type = decode_bug(regs->ip, &imm);
+	ud_type = decode_bug(regs->ip, &ud_imm, &ud_len);
 	if (ud_type == BUG_NONE)
 		return handled;
 
@@ -281,15 +301,28 @@ static noinstr bool handle_bug(struct pt
 	 */
 	if (regs->flags & X86_EFLAGS_IF)
 		raw_local_irq_enable();
-	if (ud_type == BUG_UD2) {
+
+	switch (ud_type) {
+	case BUG_UD2:
 		if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN ||
 		    handle_cfi_failure(regs) == BUG_TRAP_TYPE_WARN) {
-			regs->ip += LEN_UD2;
+			regs->ip += ud_len;
 			handled = true;
 		}
-	} else if (IS_ENABLED(CONFIG_UBSAN_TRAP)) {
-		pr_crit("%s at %pS\n", report_ubsan_failure(regs, imm), (void *)regs->ip);
+		break;
+
+	case BUG_UD1_UBSAN:
+		if (IS_ENABLED(CONFIG_UBSAN_TRAP)) {
+			pr_crit("%s at %pS\n",
+				report_ubsan_failure(regs, ud_imm),
+				(void *)regs->ip);
+		}
+		break;
+
+	default:
+		break;
 	}
+
 	if (regs->flags & X86_EFLAGS_IF)
 		raw_local_irq_disable();
 	instrumentation_end();



  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 ` Peter Zijlstra [this message]
2024-11-05 15:19   ` [PATCH 5/8] x86/traps: Cleanup and robustify decode_bug() Andrew Cooper
2024-11-05 11:39 ` [PATCH 6/8] x86/ibt: Clean up is_endbr() Peter Zijlstra
2024-11-05 11:39 ` [PATCH 7/8] x86/ibt: Clean up poison_endbr() Peter Zijlstra
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.285032152@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.