On Tue, Feb 18, 2025 at 05:48:00PM +0100, Borislav Petkov wrote:
> On Tue, Feb 18, 2025 at 03:15:35PM +0100, Peter Zijlstra wrote:
> > diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
> > index dac4d64dfb2a..cfdae25eacd7 100644
> > --- a/arch/x86/kernel/cpu/mce/severity.c
> > +++ b/arch/x86/kernel/cpu/mce/severity.c
> > @@ -301,18 +301,19 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
> > instrumentation_end();
> >
> > switch (fixup_type) {
> > - case EX_TYPE_UACCESS:
> > - if (!copy_user)
> > - return IN_KERNEL;
> > - m->kflags |= MCE_IN_KERNEL_COPYIN;
> > - fallthrough;
> > -
> > case EX_TYPE_FAULT_MCE_SAFE:
> > case EX_TYPE_DEFAULT_MCE_SAFE:
> > m->kflags |= MCE_IN_KERNEL_RECOV;
> > return IN_KERNEL_RECOV;
> >
> > default:
> > + if (copy_user) {
>
> As said on chat, if we can make is_copy_from_user() *always* correctly detect
> user access, then sure but I'm afraid EX_TYPE_UACCESS being generated at the
> handful places where we do user memory access is there for a reason as it
> makes it pretty explicit.
Thing is, we have copy routines that do not know if its user or not.
is_copy_from_user() must be reliable.
Anyway, if you all really want to go all funny, try the below.
Someone has to go and stick some EX_FLAG_USER on things, but I just
really don't believe that's doing to be useful. Because while you're
doing that, you should also audit if is_copy_from_user() will catch it
and if it does, you don't need the tag.
See how much tags you end up with..
---
diff --git a/arch/x86/include/asm/extable_fixup_types.h b/arch/x86/include/asm/extable_fixup_types.h
index 906b0d5541e8..1d6c6ff51d28 100644
--- a/arch/x86/include/asm/extable_fixup_types.h
+++ b/arch/x86/include/asm/extable_fixup_types.h
@@ -31,6 +31,9 @@
#define EX_FLAG_CLEAR_DX EX_DATA_FLAG(2)
#define EX_FLAG_CLEAR_AX_DX EX_DATA_FLAG(3)
+#define EX_FLAG_USER EX_DATA_FLAG(4)
+#define EX_FLAG_MCE EX_DATA_FLAG(8)
+
/* types */
#define EX_TYPE_NONE 0
#define EX_TYPE_DEFAULT 1
@@ -46,8 +49,6 @@
#define EX_TYPE_RDMSR_SAFE 11 /* reg := -EIO */
#define EX_TYPE_WRMSR_IN_MCE 12
#define EX_TYPE_RDMSR_IN_MCE 13
-#define EX_TYPE_DEFAULT_MCE_SAFE 14
-#define EX_TYPE_FAULT_MCE_SAFE 15
#define EX_TYPE_POP_REG 16 /* sp += sizeof(long) */
#define EX_TYPE_POP_ZERO (EX_TYPE_POP_REG | EX_DATA_IMM(0))
diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
index dac4d64dfb2a..86a32fa020d2 100644
--- a/arch/x86/kernel/cpu/mce/severity.c
+++ b/arch/x86/kernel/cpu/mce/severity.c
@@ -300,21 +300,20 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
copy_user = is_copy_from_user(regs);
instrumentation_end();
- switch (fixup_type) {
- case EX_TYPE_UACCESS:
- if (!copy_user)
- return IN_KERNEL;
- m->kflags |= MCE_IN_KERNEL_COPYIN;
- fallthrough;
-
- case EX_TYPE_FAULT_MCE_SAFE:
- case EX_TYPE_DEFAULT_MCE_SAFE:
+ if (fixup_type == EX_TYPE_NONE)
+ return IN_KERNEL;
+
+ if (fixup_type & EX_FLAG_MCE) {
m->kflags |= MCE_IN_KERNEL_RECOV;
return IN_KERNEL_RECOV;
+ }
- default:
- return IN_KERNEL;
+ if ((fixup_type & EX_FLAG_USER) || copy_user) {
+ m->kflags |= MCE_IN_KERNEL_COPYIN | MCE_IN_KERNEL_RECOV;
+ return IN_KERNEL_RECOV;
}
+
+ return IN_KERNEL;
}
/* See AMD PPR(s) section Machine Check Error Handling. */
diff --git a/arch/x86/kernel/fpu/legacy.h b/arch/x86/kernel/fpu/legacy.h
index 098f367bb8a7..3f6036840d65 100644
--- a/arch/x86/kernel/fpu/legacy.h
+++ b/arch/x86/kernel/fpu/legacy.h
@@ -24,7 +24,7 @@ static inline void ldmxcsr(u32 mxcsr)
asm volatile(ASM_STAC "\n" \
"1: " #insn "\n" \
"2: " ASM_CLAC "\n" \
- _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
+ _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT | EX_FLAG_MCE) \
: [err] "=a" (err), output \
: "0"(0), input); \
err; \
diff --git a/arch/x86/kernel/fpu/xstate.h b/arch/x86/kernel/fpu/xstate.h
index aa16f1a1bbcf..eef534091105 100644
--- a/arch/x86/kernel/fpu/xstate.h
+++ b/arch/x86/kernel/fpu/xstate.h
@@ -115,7 +115,7 @@ static inline int update_pkru_in_sigframe(struct xregs_state __user *buf, u64 ma
asm volatile("1:" op "\n\t" \
"xor %[err], %[err]\n" \
"2:\n\t" \
- _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT_MCE_SAFE) \
+ _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_FAULT | EX_FLAG_MCE) \
: [err] "=a" (err) \
: "D" (st), "m" (*st), "a" (lmask), "d" (hmask) \
: "memory")
diff --git a/arch/x86/lib/copy_mc_64.S b/arch/x86/lib/copy_mc_64.S
index c859a8a09860..7977689ad46e 100644
--- a/arch/x86/lib/copy_mc_64.S
+++ b/arch/x86/lib/copy_mc_64.S
@@ -103,9 +103,9 @@ SYM_FUNC_START(copy_mc_fragile)
movl %ecx, %edx
jmp copy_mc_fragile_handle_tail
- _ASM_EXTABLE_TYPE(.L_read_leading_bytes, .E_leading_bytes, EX_TYPE_DEFAULT_MCE_SAFE)
- _ASM_EXTABLE_TYPE(.L_read_words, .E_read_words, EX_TYPE_DEFAULT_MCE_SAFE)
- _ASM_EXTABLE_TYPE(.L_read_trailing_bytes, .E_trailing_bytes, EX_TYPE_DEFAULT_MCE_SAFE)
+ _ASM_EXTABLE_TYPE(.L_read_leading_bytes, .E_leading_bytes, EX_TYPE_DEFAULT | EX_FLAG_MCE)
+ _ASM_EXTABLE_TYPE(.L_read_words, .E_read_words, EX_TYPE_DEFAULT | EX_FLAG_MCE)
+ _ASM_EXTABLE_TYPE(.L_read_trailing_bytes, .E_trailing_bytes, EX_TYPE_DEFAULT | EX_FLAG_MCE)
_ASM_EXTABLE(.L_write_leading_bytes, .E_leading_bytes)
_ASM_EXTABLE(.L_write_words, .E_write_words)
_ASM_EXTABLE(.L_write_trailing_bytes, .E_trailing_bytes)
@@ -143,7 +143,7 @@ SYM_FUNC_START(copy_mc_enhanced_fast_string)
movq %rcx, %rax
RET
- _ASM_EXTABLE_TYPE(.L_copy, .E_copy, EX_TYPE_DEFAULT_MCE_SAFE)
+ _ASM_EXTABLE_TYPE(.L_copy, .E_copy, EX_TYPE_DEFAULT | EX_FLAG_MCE)
SYM_FUNC_END(copy_mc_enhanced_fast_string)
#endif /* !CONFIG_UML */
diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
index 51986e8a9d35..7358bf10baba 100644
--- a/arch/x86/mm/extable.c
+++ b/arch/x86/mm/extable.c
@@ -293,8 +293,10 @@ static bool ex_handler_eretu(const struct exception_table_entry *fixup,
int ex_get_fixup_type(unsigned long ip)
{
const struct exception_table_entry *e = search_exception_tables(ip);
+ if (!e)
+ return EX_TYPE_NONE;
- return e ? FIELD_GET(EX_DATA_TYPE_MASK, e->data) : EX_TYPE_NONE;
+ return FIELD_GET(EX_DATA_TYPE_MASK, e->data) | (e->data & (EX_FLAG_USER | EX_FLAG_MCE));
}
int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
@@ -327,10 +329,8 @@ int fixup_exception(struct pt_regs *regs, int trapnr, unsigned long error_code,
switch (type) {
case EX_TYPE_DEFAULT:
- case EX_TYPE_DEFAULT_MCE_SAFE:
return ex_handler_default(e, regs);
case EX_TYPE_FAULT:
- case EX_TYPE_FAULT_MCE_SAFE:
return ex_handler_fault(e, regs, trapnr);
case EX_TYPE_UACCESS:
return ex_handler_uaccess(e, regs, trapnr, fault_addr);
