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) <[email protected]> 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
