x86/traps: Cleanup and robustify decode_bug()

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>
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Link: https://lore.kernel.org/r/20250207122546.721120726@infradead.org

diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index e85ac0c7c039ea95272a5525e2099ccadf386c1f..1a5e4b3726940471b069121410cd88e99080bd65 100644 (file)
--- 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
 

diff --git a/arch/x86/include/asm/ibt.h b/arch/x86/include/asm/ibt.h
index d955e0d1cbf15ff43ef5c093f100a3a639cf92a4..f0ca5c0b1a6b2842332c98a39a2ac493a4e721ac 100644 (file)
--- 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 gen_endbr(void)
        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

diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 2dbadf347b5f4f66625c4f49b76c41b412270d57..05b86c05e446c34722c5d6bc875075173b580297 100644 (file)
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -94,10 +94,17 @@ __always_inline int is_valid_bugaddr(unsigned long addr)
 
 /*
  * 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,42 @@ __always_inline int decode_bug(unsigned long addr, u32 *imm)
                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 */
+
+       if (X86_MODRM_MOD(v) != 3 && X86_MODRM_RM(v) == 4)
+               addr++;                 /* SIB */
+
+       /* Decode immediate, if present */
+       switch (X86_MODRM_MOD(v)) {
+       case 0: if (X86_MODRM_RM(v) == 5)
+                       addr += 4; /* RIP + disp32 */
+               break;
+
+       case 1: *imm = *(s8 *)addr;
+               addr += 1;
+               break;
+
+       case 2: *imm = *(s32 *)addr;
+               addr += 4;
+               break;
+
+       case 3: break;
+       }
+
+       /* record instruction length */
+       *len = addr - start;
+
+       if (X86_MODRM_REG(v) == 0)      /* EAX */
+               return BUG_UD1_UBSAN;
 
        return BUG_UD1;
 }
@@ -258,10 +283,10 @@ static inline void handle_invalid_op(struct pt_regs *regs)
 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 +306,28 @@ static noinstr bool handle_bug(struct pt_regs *regs)
         */
        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();