Re: [PATCH v14 01/14] x86/mce: Fix hardware debug register corruption on task migration

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving local_db_save() inside exc_machine_check_kernel() exposes early #MC entry code to hardware breakpoints, partially reverting commit cd840e424f27 and allowing nested #DB exceptions that can fatally corrupt the #DB IST stack.
--

commit 22e84ef34bc3d02a04e2e3356bffad4d11749a7a
Author: Masami Hiramatsu (Google) <mhiramat@kernel.org>

x86/mce: Fix hardware debug register corruption on task migration

In exc_machine_check_user(), local_db_save() and local_db_restore() are
invoked in the outer entry stubs, surrounding exc_machine_check_user().
However, exc_machine_check_user() handles pending thread work and may
schedule(), causing the task to migrate to another CPU. This patch moves
local_db_save() and local_db_restore() inside exc_machine_check_user()
and exc_machine_check_kernel() to prevent hardware debug register corruption.

> diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
> index cfb74be19994d..61ade8b5c9d7b 100644
> --- a/arch/x86/kernel/cpu/mce/core.c
> +++ b/arch/x86/kernel/cpu/mce/core.c
> @@ -2108,6 +2108,7 @@ bool filter_mce(struct mce *m)
>  static __always_inline void exc_machine_check_kernel(struct pt_regs *regs)
>  {
>  	irqentry_state_t irq_state;
> +	unsigned long dr7;
>  
>  	WARN_ON_ONCE(user_mode(regs));
>  
> @@ -2118,18 +2119,24 @@ static __always_inline void exc_machine_check_kernel(struct pt_regs *regs)
>  	if (mca_cfg.initialized && mce_check_crashing_cpu())
>  		return;
>  
> +	dr7 = local_db_save();

[Severity: High]
Does moving local_db_save() below the early checks in
exc_machine_check_kernel() expose this early #MC entry code to hardware
breakpoints?

If a privileged user sets a hardware data breakpoint on a kernel variable
accessed here, such as mca_cfg.initialized or variables inside
mce_check_crashing_cpu(), a #DB exception could fire in kernel mode.

If a Machine Check Exception (#MC) then occurs and preempts the #DB handler,
it switches to the #MC IST stack. Because local_db_save() is not called
until after these variable accesses, the code could trigger a nested #DB
exception.

Since x86 uses a fixed-size Interrupt Stack Table (IST) for #DB exceptions
that unconditionally resets the stack pointer to the top of the #DB IST
stack, wouldn't the nested #DB overwrite the active stack frame of the
interrupted #DB handler, leading to a fatal crash when it attempts to
return?

Could local_db_save() be called at the very beginning of
exc_machine_check_kernel() to prevent this IST stack corruption?

>  	irq_state = irqentry_nmi_enter(regs);
>  
>  	do_machine_check(regs);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178810001186.64882.2161016469449127450.stgit@devnote2?part=1