Messages in this thread Patch in this message | | | Date | Thu, 31 Mar 2022 19:31:25 +0800 | From | Youquan Song <> | 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@intel.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@vger.kernel.org> # v5.17+ Signed-off-by: Zhiquan Li <zhiquan1.li@intel.com> Co-developed-by: Youquan Song <youquan.song@intel.com> Signed-off-by: Youquan Song <youquan.song@intel.com> Tested-by: Tony Luck <tony.luck@intel.com> Signed-off-by: Tony Luck <tony.luck@intel.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
| |