On Sun, Feb 04, 2024 at 04:26:26PM +0800, Tong Tiangen wrote:
> diff --git a/arch/x86/kernel/cpu/mce/severity.c b/arch/x86/kernel/cpu/mce/severity.c
> index bca780fa5e57..b2cce1b6c96d 100644
> --- a/arch/x86/kernel/cpu/mce/severity.c
> +++ b/arch/x86/kernel/cpu/mce/severity.c
> @@ -292,11 +292,11 @@ static noinstr int error_context(struct mce *m, struct pt_regs *regs)
> case EX_TYPE_UACCESS:
> if (!copy_user)
> return IN_KERNEL;
> + fallthrough;
> + case EX_TYPE_DEFAULT_MCE_SAFE:
> m->kflags |= MCE_IN_KERNEL_COPYIN;
> fallthrough;
I knew something was still bugging me here and this is still wrong.
Let's imagine this flow:
copy_mc_to_user() - note *src is kernel memory
|-> copy_mc_enhanced_fast_string or copy_mc_fragile - it's the same thing
|-> -#MC, exception type EX_TYPE_DEFAULT_MCE_SAFE
|-> error_context():
case EX_TYPE_DEFAULT_MCE_SAFE:
m->kflags |= MCE_IN_KERNEL_COPYIN;
MCE_IN_KERNEL_COPYIN does kill_me_never():
pr_err("Kernel accessed poison in user space at %llx\n", p->mce_addr);
but that's reading from kernel memory!
IOW, I *think* that switch statement should be this:
switch (fixup_type) {
case EX_TYPE_UACCESS:
case EX_TYPE_DEFAULT_MCE_SAFE:
if (!copy_user)
return IN_KERNEL;
m->kflags |= MCE_IN_KERNEL_COPYIN;
fallthrough;
case EX_TYPE_FAULT_MCE_SAFE:
m->kflags |= MCE_IN_KERNEL_RECOV;
return IN_KERNEL_RECOV;
default:
return IN_KERNEL;
}
Provided I'm not missing a case and provided is_copy_from_user() really
detects all cases properly.
And then patch 3 is wrong because we only can handle "copy in" - not
just any copy.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
