[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220331113125.GA2628@linux-youquan.bj.intel.com>
Date: Thu, 31 Mar 2022 19:31:25 +0800
From: Youquan Song <youquan.song@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Tony Luck <tony.luck@...el.com>, Borislav Petkov <bp@...en8.de>,
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>
Subject: Re: [PATCH] x86/uaccess: restore get_user exception type to
EX_TYPE_UACCESS
> Did you do your testing on RHEL or something daft like that?
Tested on RHEL8.x
> Something like the below can also work, I suppose. But please, add
> coherent comments to the extable code with useful references to the MCE
> code that does this abuse.
Here is the full fix patch depending on your suggestion. Thanks a lot!
>From 3d2e44dac774d49417002136ee3b007638a85191 Mon Sep 17 00:00:00 2001
From: Zhiquan Li <zhiquan1.li@...el.com>
Date: Wed, 30 Mar 2022 21:11:06 +0800
Subject: [PATCH v2] x86/uaccess: identify get_user with new type
EX_TYPE_UA_IMM_REG
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
the opportunity to rescue the system.
To fix it we have to add a new exception type for get_user,
EX_TYPE_UA_IMM_REG, 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
compound type EX_TYPE_UFAULT_REG to figure out this case, it will not
break anything. Then MCE handler can identify is as a recoverable case.
Changes v1 -> v2:
* Abandoned the solution that restoring get_user exception type to
EX_TYPE_UACCESS and adding new flag EX_FLAG_SET_REG to set
reg := imm additionally.
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>
---
arch/x86/include/asm/extable_fixup_types.h | 3 +++
arch/x86/include/asm/uaccess.h | 6 +++---
arch/x86/kernel/cpu/mce/severity.c | 1 +
arch/x86/mm/extable.c | 1 +
4 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 503622627400..759283acb246 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -64,4 +64,7 @@
#define EX_TYPE_UCOPY_LEN4 (EX_TYPE_UCOPY_LEN | EX_DATA_IMM(4))
#define EX_TYPE_UCOPY_LEN8 (EX_TYPE_UCOPY_LEN | EX_DATA_IMM(8))
+#define EX_TYPE_UA_IMM_REG 20 /* reg := (long)imm */
+#define EX_TYPE_UFAULT_REG (EX_TYPE_UA_IMM_REG | EX_DATA_IMM(-EFAULT))
+
#endif
diff --git a/arch/x86/include/asm/uaccess.h b/arch/x86/include/asm/uaccess.h
index ac96f9b2d64b..2b3a4bb609fe 100644
--- a/arch/x86/include/asm/uaccess.h
+++ b/arch/x86/include/asm/uaccess.h
@@ -352,10 +352,10 @@ do { \
"1: movl %[lowbits],%%eax\n" \
"2: movl %[highbits],%%edx\n" \
"3:\n" \
- _ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_EFAULT_REG | \
+ _ASM_EXTABLE_TYPE_REG(1b, 3b, EX_TYPE_UFAULT_REG | \
EX_FLAG_CLEAR_AX_DX, \
%[errout]) \
- _ASM_EXTABLE_TYPE_REG(2b, 3b, EX_TYPE_EFAULT_REG | \
+ _ASM_EXTABLE_TYPE_REG(2b, 3b, EX_TYPE_UFAULT_REG | \
EX_FLAG_CLEAR_AX_DX, \
%[errout]) \
: [errout] "=r" (retval), \
@@ -399,7 +399,7 @@ do { \
asm volatile("\n" \
"1: mov"itype" %[umem],%[output]\n" \
"2:\n" \
- _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_EFAULT_REG | \
+ _ASM_EXTABLE_TYPE_REG(1b, 2b, EX_TYPE_UFAULT_REG | \
EX_FLAG_CLEAR_AX, \
%[errout]) \
: [errout] "=r" (err), \
diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
index 7aa2bda93cbb..a20bb930b322 100644
--- a/arch/x86/kernel/cpu/mce/severity.c
+++ b/arch/x86/kernel/cpu/mce/severity.c
@@ -286,6 +286,7 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
switch (fixup_type) {
case EX_TYPE_UACCESS:
case EX_TYPE_COPY:
+ case EX_TYPE_UA_IMM_REG:
if (!copy_user)
return IN_KERNEL;
m->kflags |= MCE_IN_KERNEL_COPYIN;
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index dba2197c05c3..b9bc0e7cb73e 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -210,6 +210,7 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
regs->sp += sizeof(long);
fallthrough;
case EX_TYPE_IMM_REG:
+ case EX_TYPE_UA_IMM_REG:
return ex_handler_imm_reg(e, regs, reg, imm);
case EX_TYPE_FAULT_SGX:
return ex_handler_sgx(e, regs, trapnr);
--
2.25.1
Powered by blists - more mailing lists