On Tue, Feb 18, 2025 at 09:28:33PM +0800, Shuai Xue wrote:
> I did build and test this patch set on it. But I did not find any warnings.
> Could you provide more details?
NOINSTR_VALIDATION=y helps
> > > /* Allow instrumentation around external facilities usage. */
> > > instrumentation_begin();
> > > - fixup_type = ex_get_fixup_type(m->ip);
> > > + fixup_type = FIELD_GET(EX_DATA_TYPE_MASK, e->data);
> > > + imm = FIELD_GET(EX_DATA_IMM_MASK, e->data);
> > > copy_user = is_copy_from_user(regs);
> > > instrumentation_end();
> > > @@ -304,9 +311,13 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
> > > case EX_TYPE_UACCESS:
> > > if (!copy_user)
> > > return IN_KERNEL;
> > > - m->kflags |= MCE_IN_KERNEL_COPYIN;
> > > - fallthrough;
> > > -
> > > + m->kflags |= MCE_IN_KERNEL_COPYIN | MCE_IN_KERNEL_RECOV;
> > > + return IN_KERNEL_RECOV;
> > > + case EX_TYPE_IMM_REG:
> > > + if (!copy_user || imm != -EFAULT)
> > > + return IN_KERNEL;
> > > + m->kflags |= MCE_IN_KERNEL_COPYIN | MCE_IN_KERNEL_RECOV;
> > > + return IN_KERNEL_RECOV;
> >
> > Maybe I'm justnot understanding things, but what's wrong with something
> > like the below; why do we care about the ex-type if we know its a MOV
> > reading from userspace?
> >
> > The less we muck about with the extable here, the better.
>
> We need to make sure that we have register a fixup handler for the copy_user
> case. If no fixup handler found, the PC accessing posion will trigger #MCE
> again and again resulting a hardlock up.
Well, then write it like so. Afaict, you don't care what the actual
exception type is, just that there is one, for the copy_user case.
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) {
+ m->kflags |= MCE_IN_KERNEL_COPYIN | MCE_IN_KERNEL_RECOV;
+ return IN_KERNEL_RECOV;
+ }
+ fallthrough;
+
+ case EX_TYPE_NONE:
return IN_KERNEL;
}
}
