linux-kernel - [PATCH] x86/uaccess: restore get_user exception type to EX_TYPE

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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20220328201748.1864491-1-tony.luck@intel.com>
Date:   Mon, 28 Mar 2022 13:17:48 -0700
From:   Tony Luck <tony.luck@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Josh Poimboeuf <jpoimboe@...hat.com>, x86@...nel.org,
        linux-kernel@...r.kernel.org, Zhiquan Li <zhiquan1.li@...el.com>,
        Youquan Song <youquan.song@...el.com>,
        Tony Luck <tony.luck@...el.com>
Subject: [PATCH] x86/uaccess: restore get_user exception type to EX_TYPE_UACCESS

From: Zhiquan Li <zhiquan1.li@...el.com>

5.17.0 kernel will crash when we inject MCE by run "einj_mem_uc copyin"
in ras-tools with CONFIG_CC_HAS_ASM_GOTO_OUTPUT != y kernel config.
mce: [Hardware Error]: Machine check events logged
mce: [Hardware Error]: CPU 120: Machine Check Exception: f Bank 1: bd80000000100134
mce: [Hardware Error]: RIP 10: {fault_in_readable+0x9f/0xd0}
mce: [Hardware Error]: TSC 63d3fa6181b69 ADDR f921f31400 MISC 86 PPIN 11a090eb80bf0c9c
mce: [Hardware Error]: PROCESSOR 0:606a6 TIME 1647365323 SOCKET 1 APIC 8d microcode d0002e0
mce: [Hardware Error]: Run the above through 'mcelog --ascii'
mce: [Hardware Error]: Machine check: Data load in unrecoverable area of kernel
Kernel panic - not syncing: Fatal local machine check

In commit 99641e094d6c ("x86/uaccess: Remove .fixup usage"), the
exception type of get_user was changed from EX_TYPE_UACCESS to
EX_TYPE_EFAULT_REG. In case of MCE/SRAR when kernel copy data from user,
the MCE handler identities the exception type with EX_TYPE_UACCESS to
MCE_IN_KERNEL_RECOV. While the new type EX_TYPE_EFAULT_REG will lose
lose the opportunity to rescue the system.

To fix it we have to restore get_user exception type to EX_TYPE_UACCESS,
but we do not want to miss the operation that make reg = -EFAULT in
function ex_handler_imm_reg(), so we so add a new flag EX_FLAG_SET_REG
to identify this case, it will not break anything.

Fixes: 99641e094d6c ("x86/uaccess: Remove .fixup usage")
Cc: <stable@...r.kernel.org> # v5.17+
Signed-off-by: Zhiquan Li <zhiquan1.li@...el.com>
Co-developed-by: Youquan Song <youquan.song@...el.com>
Signed-off-by: Youquan Song <youquan.song@...el.com>
Tested-by: Tony Luck <tony.luck@...el.com>
Signed-off-by: Tony Luck <tony.luck@...el.com>
---

This patch works ... but to test it I had to fake out init/Kconfig so
that it wouldn't set CONFIG_CC_HAS_ASM_GOTO_OUTPUT=y. So it seems that
this is only needed when building with some old compiler version.

With Linus' announcement about C99/C11 as new basis, is this fix
needed? I.e. is it still valid to build the upstream kernel with a
compiler that doesn't grok CONFIG_CC_HAS_ASM_GOTO_OUTPUT?

 arch/x86/include/asm/extable_fixup_types.h |  1 +
 arch/x86/include/asm/uaccess.h             | 15 +++++++++------
 arch/x86/mm/extable.c                      |  8 ++++++++
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 503622627400..329eeebba2f6 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -30,6 +30,7 @@
 #define EX_FLAG_CLEAR_AX		EX_DATA_FLAG(1)
 #define EX_FLAG_CLEAR_DX		EX_DATA_FLAG(2)
 #define EX_FLAG_CLEAR_AX_DX		EX_DATA_FLAG(3)
+#define EX_FLAG_SET_REG		EX_DATA_FLAG(4)
 
 /* types */
 #define	EX_TYPE_NONE			 0
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index f78e2b3501a1..277f7c87ad81 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -325,11 +325,13 @@ do {									\
 		     "1:	movl %[lowbits],%%eax\n"		\
 		     "2:	movl %[highbits],%%edx\n"		\
 		     "3:\n"						\
-		     _ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_EFAULT_REG |	\
-					   EX_FLAG_CLEAR_AX_DX,		\
+		     _ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_UACCESS |	\
+					   EX_DATA_IMM(-EFAULT) | \
+					   EX_FLAG_CLEAR_AX_DX | EX_FLAG_SET_REG,	\
 					   %[errout])			\
-		     _ASM_EXTABLE_TYPE_REG(2b, 3b, EX_TYPE_EFAULT_REG |	\
-					   EX_FLAG_CLEAR_AX_DX,		\
+		     _ASM_EXTABLE_TYPE_REG(2b, 3b, EX_TYPE_UACCESS |	\
+					   EX_DATA_IMM(-EFAULT) | \
+					   EX_FLAG_CLEAR_AX_DX | EX_FLAG_SET_REG,	\
 					   %[errout])			\
 		     : [errout] "=r" (retval),				\
 		       [output] "=&A"(x)				\
@@ -372,8 +374,9 @@ do {									\
 	asm volatile("\n"						\
 		     "1:	mov"itype" %[umem],%[output]\n"		\
 		     "2:\n"						\
-		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG | \
-					   EX_FLAG_CLEAR_AX,		\
+		     _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_UACCESS |	\
+					   EX_DATA_IMM(-EFAULT) | \
+					   EX_FLAG_CLEAR_AX | EX_FLAG_SET_REG,		\
 					   %[errout])			\
 		     : [errout] "=r" (err),				\
 		       [output] "=a" (x)				\
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index dba2197c05c3..aac599781879 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -183,6 +183,14 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
 	case EX_TYPE_FAULT_MCE_SAFE:
 		return ex_handler_fault(e, regs, trapnr);
 	case EX_TYPE_UACCESS:
+		/*
+		 * In the user access case, we might need err reg = -EFAULT, like:
+		 * _ASM_EXTABLE_TYPE_REG(from, to, EX_TYPE_UACCESS | \
+		 *                            EX_DATA_IMM(-EFAULT) | \
+		 *                            EX_FLAG_SET_REG, reg)
+		 */
+		if (e->data & EX_FLAG_SET_REG)
+			*pt_regs_nr(regs, reg) = (long)imm;
 		return ex_handler_uaccess(e, regs, trapnr);
 	case EX_TYPE_COPY:
 		return ex_handler_copy(e, regs, trapnr);
-- 
2.35.1