linux-kernel - [RFC][PATCH 6/6] x86/mce: Dont use noinstr for now

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210106144017.779081583@infradead.org>
Date:   Wed, 06 Jan 2021 15:36:25 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     x86@...nel.org
Cc:     linux-kernel@...r.kernel.org, peterz@...radead.org,
        rdunlap@...radead.org, jpoimboe@...hat.com, sfr@...b.auug.org.au,
        tony.luck@...el.com
Subject: [RFC][PATCH 6/6] x86/mce: Dont use noinstr for now

We call do_machine_check() from a 'safe' context, instrumentation
should be possible. However, due to the nature of the beast, MCE is
fragile and we really rather would not want to hit additional code.

Currently this is done by marking do_machine_check() as noinstr,
however there's at least one objtool warning because of that:

  vmlinux.o: warning: objtool: do_machine_check()+0x650: call to queue_task_work() leaves .noinstr.text section

trying to fix that leads down a rabbit-hole. A _lot_ of code would end
up having to be marked noinstr outside of the MCE code. While
possible, doing so is undesired because it would hide it from
instrumentation for all other users.

Instead of using noinstr, kill instrumentation for all of mce/. This
switches MCE over to a best-effort but non-validated mode. Doing
better will require a significant investment of time and effort.

The obvious problem with tracing the MCE is that the tracer will
trigger a further MCE and we DIE an instant death.

Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
---
 arch/x86/kernel/cpu/mce/Makefile |    7 +++++++
 arch/x86/kernel/cpu/mce/core.c   |    6 +-----
 2 files changed, 8 insertions(+), 5 deletions(-)

--- a/arch/x86/kernel/cpu/mce/Makefile
+++ b/arch/x86/kernel/cpu/mce/Makefile
@@ -1,4 +1,11 @@
 # SPDX-License-Identifier: GPL-2.0
+
+# No instrumentation for MCE code
+KASAN_SANITIZE := n
+UBSAN_SANITIZE := n
+KCOV_INSTRUMENT := n
+ccflags-remove-$(CONFIG_FUNCTION_TRACER) += $(CC_FLAGS_FTRACE)
+
 obj-y				=  core.o severity.o genpool.o

 obj-$(CONFIG_X86_ANCIENT_MCE)	+= winchip.o p5.o
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c
@@ -1299,7 +1299,7 @@ static void queue_task_work(struct mce *
  * backing the user stack, tracing that reads the user stack will cause
  * potentially infinite recursion.
  */
-noinstr void do_machine_check(struct pt_regs *regs)
+void do_machine_check(struct pt_regs *regs)
 {
 	DECLARE_BITMAP(valid_banks, MAX_NR_BANKS);
 	DECLARE_BITMAP(toclear, MAX_NR_BANKS);
@@ -1987,10 +1987,6 @@ static __always_inline void exc_machine_
 		return;

 	irq_state = irqentry_nmi_enter(regs);
-	/*
-	 * The call targets are marked noinstr, but objtool can't figure
-	 * that out because it's an indirect call. Annotate it.
-	 */
 	instrumentation_begin();

 	machine_check_vector(regs);