| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240903081748.GN4723@noisy.programming.kicks-ass.net>
Date: Tue, 3 Sep 2024 10:17:48 +0200
From: Peter Zijlstra <peterz@...radead.org>
To: Josh Poimboeuf <jpoimboe@...nel.org>
Cc: live-patching@...r.kernel.org, linux-kernel@...r.kernel.org,
x86@...nel.org, Miroslav Benes <mbenes@...e.cz>,
Petr Mladek <pmladek@...e.com>,
Joe Lawrence <joe.lawrence@...hat.com>,
Jiri Kosina <jikos@...nel.org>,
Marcos Paulo de Souza <mpdesouza@...e.com>,
Song Liu <song@...nel.org>
Subject: Re: [RFC 20/31] objtool: Add UD1 detection
On Mon, Sep 02, 2024 at 09:00:03PM -0700, Josh Poimboeuf wrote:
> A UD1 isn't a BUG and shouldn't be treated like one.
>
> Signed-off-by: Josh Poimboeuf <jpoimboe@...nel.org>
> ---
> tools/objtool/arch/x86/decode.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
> index 6b34b058a821..72d55dcd3d7f 100644
> --- a/tools/objtool/arch/x86/decode.c
> +++ b/tools/objtool/arch/x86/decode.c
> @@ -528,11 +528,19 @@ int arch_decode_instruction(struct objtool_file *file, const struct section *sec
> /* sysenter, sysret */
> insn->type = INSN_CONTEXT_SWITCH;
>
> - } else if (op2 == 0x0b || op2 == 0xb9) {
> + } else if (op2 == 0x0b) {
>
> /* ud2 */
> insn->type = INSN_BUG;
>
> + } else if (op2 == 0xb9) {
> +
> + /*
> + * ud1 - only used for the static call trampoline to
> + * stop speculation. Basically used like an int3.
> + */
> + insn->type = INSN_TRAP;
> +
> } else if (op2 == 0x0d || op2 == 0x1f) {
>
> /* nopl/nopw */
We recently grew more UD1 usage...
---
commit 7424fc6b86c8980a87169e005f5cd4438d18efe6
Author: Gatlin Newhouse <gatlin.newhouse@...il.com>
Date: Wed Jul 24 00:01:55 2024 +0000
x86/traps: Enable UBSAN traps on x86
Currently ARM64 extracts which specific sanitizer has caused a trap via
encoded data in the trap instruction. Clang on x86 currently encodes the
same data in the UD1 instruction but x86 handle_bug() and
is_valid_bugaddr() currently only look at UD2.
Bring x86 to parity with ARM64, similar to commit 25b84002afb9 ("arm64:
Support Clang UBSAN trap codes for better reporting"). See the llvm
links for information about the code generation.
Enable the reporting of UBSAN sanitizer details on x86 compiled with clang
when CONFIG_UBSAN_TRAP=y by analysing UD1 and retrieving the type immediate
which is encoded by the compiler after the UD1.
[ tglx: Simplified it by moving the printk() into handle_bug() ]
Signed-off-by: Gatlin Newhouse <gatlin.newhouse@...il.com>
Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>
Cc: Kees Cook <keescook@...omium.org>
Link: https://lore.kernel.org/all/20240724000206.451425-1-gatlin.newhouse@gmail.com
Link: https://github.com/llvm/llvm-project/commit/c5978f42ec8e9#diff-bb68d7cd885f41cfc35843998b0f9f534adb60b415f647109e597ce448e92d9f
Link: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Target/X86/X86InstrSystem.td#L27
diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h
index a3ec87d198ac..806649c7f23d 100644
--- a/arch/x86/include/asm/bug.h
+++ b/arch/x86/include/asm/bug.h
@@ -13,6 +13,18 @@
#define INSN_UD2 0x0b0f
#define LEN_UD2 2
+/*
+ * In clang we have UD1s reporting UBSAN failures on X86, 64 and 32bit.
+ */
+#define INSN_ASOP 0x67
+#define OPCODE_ESCAPE 0x0f
+#define SECOND_BYTE_OPCODE_UD1 0xb9
+#define SECOND_BYTE_OPCODE_UD2 0x0b
+
+#define BUG_NONE 0xffff
+#define BUG_UD1 0xfffe
+#define BUG_UD2 0xfffd
+
#ifdef CONFIG_GENERIC_BUG
#ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
index 4fa0b17e5043..415881607c5d 100644
--- a/arch/x86/kernel/traps.c
+++ b/arch/x86/kernel/traps.c
@@ -42,6 +42,7 @@
#include <linux/hardirq.h>
#include <linux/atomic.h>
#include <linux/iommu.h>
+#include <linux/ubsan.h>
#include <asm/stacktrace.h>
#include <asm/processor.h>
@@ -91,6 +92,47 @@ __always_inline int is_valid_bugaddr(unsigned long addr)
return *(unsigned short *)addr == INSN_UD2;
}
+/*
+ * 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.
+ */
+__always_inline int decode_bug(unsigned long addr, u32 *imm)
+{
+ u8 v;
+
+ if (addr < TASK_SIZE_MAX)
+ return BUG_NONE;
+
+ v = *(u8 *)(addr++);
+ if (v == INSN_ASOP)
+ v = *(u8 *)(addr++);
+ if (v != OPCODE_ESCAPE)
+ return BUG_NONE;
+
+ v = *(u8 *)(addr++);
+ if (v == SECOND_BYTE_OPCODE_UD2)
+ return BUG_UD2;
+
+ if (!IS_ENABLED(CONFIG_UBSAN_TRAP) || 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));
+
+ return BUG_UD1;
+}
+
+
static nokprobe_inline int
do_trap_no_signal(struct task_struct *tsk, int trapnr, const char *str,
struct pt_regs *regs, long error_code)
@@ -216,6 +258,8 @@ 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;
/*
* Normally @regs are unpoisoned by irqentry_enter(), but handle_bug()
@@ -223,7 +267,8 @@ static noinstr bool handle_bug(struct pt_regs *regs)
* irqentry_enter().
*/
kmsan_unpoison_entry_regs(regs);
- if (!is_valid_bugaddr(regs->ip))
+ ud_type = decode_bug(regs->ip, &imm);
+ if (ud_type == BUG_NONE)
return handled;
/*
@@ -236,10 +281,14 @@ static noinstr bool handle_bug(struct pt_regs *regs)
*/
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_enable();
- if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN ||
- handle_cfi_failure(regs) == BUG_TRAP_TYPE_WARN) {
- regs->ip += LEN_UD2;
- handled = true;
+ if (ud_type == BUG_UD2) {
+ if (report_bug(regs->ip, regs) == BUG_TRAP_TYPE_WARN ||
+ handle_cfi_failure(regs) == BUG_TRAP_TYPE_WARN) {
+ regs->ip += LEN_UD2;
+ handled = true;
+ }
+ } else if (IS_ENABLED(CONFIG_UBSAN_TRAP)) {
+ pr_crit("%s at %pS\n", report_ubsan_failure(regs, imm), (void *)regs->ip);
}
if (regs->flags & X86_EFLAGS_IF)
raw_local_irq_disable();
diff --git a/include/linux/ubsan.h b/include/linux/ubsan.h
index bff7445498de..d8219cbe09ff 100644
--- a/include/linux/ubsan.h
+++ b/include/linux/ubsan.h
@@ -4,6 +4,11 @@
#ifdef CONFIG_UBSAN_TRAP
const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type);
+#else
+static inline const char *report_ubsan_failure(struct pt_regs *regs, u32 check_type)
+{
+ return NULL;
+}
#endif
#endif
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index bdda600f8dfb..1d4aa7a83b3a 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -29,8 +29,8 @@ config UBSAN_TRAP
Also note that selecting Y will cause your kernel to Oops
with an "illegal instruction" error with no further details
- when a UBSAN violation occurs. (Except on arm64, which will
- report which Sanitizer failed.) This may make it hard to
+ when a UBSAN violation occurs. (Except on arm64 and x86, which
+ will report which Sanitizer failed.) This may make it hard to
determine whether an Oops was caused by UBSAN or to figure
out the details of a UBSAN violation. It makes the kernel log
output less useful for bug reports.
Powered by blists - more mailing lists