Re: [PATCH v13 02/12] x86/hw_breakpoints: Make DR7 updates NMI safe

On Sat, 22 Aug 2026 09:41:06 +0000
sashiko-bot@kernel.org wrote:

quoted

-static __always_inline void local_db_restore(unsigned long dr7)
+static __always_inline void local_db_restore(unsigned long dr7,
+					     unsigned int dr7_seq)
 {
+	unsigned int seq;
+
 	/*
 	 * Ensure the compiler doesn't raise this statement into
 	 * the critical section; enabling breakpoints early would
 	 * not be good.
 	 */
 	barrier();
-	if (dr7)
+
+	do {
+		seq = this_cpu_read(cpu_dr7_seq);
+		if (seq == dr7_seq) {
+			if (!dr7)
+				return;
+		} else {
+			dr7 = this_cpu_read(cpu_dr7);
+			if (!dr7)
+				dr7 = DR7_FIXED_1;
+		}
+
 		set_debugreg(dr7, 7);
+		barrier();
+	} while (unlikely(seq != this_cpu_read(cpu_dr7_seq)));
 }

[Severity: High]
If KVM intentionally clears the hardware DR7 state to isolate the guest from
host breakpoints before VMENTER without modifying the host's cpu_dr7 shadow
state:

arch/x86/kvm/x86.c:vcpu_enter_guest() {
    ...
    if (unlikely(vcpu->arch.switch_db_regs &&
                 !(vcpu->arch.switch_db_regs & KVM_DEBUGREG_AUTO_SWITCH))) {
        set_debugreg(DR7_FIXED_1, 7);
    ...
    } else if (unlikely(hw_breakpoint_active())) {
        set_debugreg(DR7_FIXED_1, 7);
    }
    ...
}

Could an NMI firing shortly after this permanently undo this isolation
upon return?

If an NMI (such as a perf event) fires before vmlaunch/vmresume, exc_nmi
calls local_db_save, saving dr7=0. If the NMI modifies a breakpoint,
cpu_dr7_seq increments.

When exc_nmi calls local_db_restore, the sequence mismatch causes it to
discard the saved dr7=0 state and instead unconditionally restore the host's
active breakpoints (this_cpu_read(cpu_dr7)) to the hardware DR7. This could
leak host hardware breakpoints into a KVM guest and cause unexpected #DB
exceptions.

OK, this seems only happen when KGDB/KDB sets the watchpoint from
NMI context (right after KVM disables dr7 for transient to guest).
We can avoid this by early returning if dr7 == 0 in local_db_restore().
(But..., if user intensively set the dr7 in that timing, it will be
ignored.)

[ ... ]

quoted

diff --git a/arch/x86/kernel/cpu/mce/core.c b/arch/x86/kernel/cpu/mce/core.c
index 9bba1e2f03af7..8dba9cd04bfa5 100644
--- a/arch/x86/kernel/cpu/mce/core.c
+++ b/arch/x86/kernel/cpu/mce/core.c

[ ... ]

quoted

@@ -2139,20 +2139,22 @@ static __always_inline void exc_machine_check_user(struct pt_regs *regs)
 DEFINE_IDTENTRY_MCE(exc_machine_check)
 {
 	unsigned long dr7;
+	unsigned int dr7_seq;
 
-	dr7 = local_db_save();
+	local_db_save(&dr7, &dr7_seq);
 	exc_machine_check_kernel(regs);
-	local_db_restore(dr7);
+	local_db_restore(dr7, dr7_seq);
 }
 
 /* The user mode variant. */
 DEFINE_IDTENTRY_MCE_USER(exc_machine_check)
 {
 	unsigned long dr7;
+	unsigned int dr7_seq;
 
-	dr7 = local_db_save();
+	local_db_save(&dr7, &dr7_seq);
 	exc_machine_check_user(regs);
-	local_db_restore(dr7);
+	local_db_restore(dr7, dr7_seq);
 }

[Severity: High]
This is a pre-existing issue, but does task migration during user-mode
machine check exceptions corrupt hardware debug registers on the new CPU?

exc_machine_check_user invokes irqentry_exit_to_user_mode, which processes
pending thread work (like TIF_NEED_RESCHED), enabling interrupts and allowing
the task to be scheduled out and migrate to a new CPU:

CPU1
exc_machine_check()
    local_db_save(&dr7, &dr7_seq);
    exc_machine_check_user(regs)
        irqentry_exit_to_user_mode(regs)
            // Task migrates to CPU2

CPU2
    local_db_restore(dr7, dr7_seq);

When it resumes on the new CPU, local_db_restore is executed with the dr7
and dr7_seq values saved from the old CPU. If the new CPU's cpu_dr7_seq
matches the old CPU's saved dr7_seq, it would erroneously write the old CPU's
hardware DR7 state to the new CPU's hardware DR7 register. Could this cause
spurious #DB exceptions or break hardware breakpoints on the new CPU?

OK, this is another problem to be fixed in a bugfix patch. We should do
local_db_save/restore() inside exc_machine_check_user() before calling
irqentry_exit_to_user_mode().

Thank you,

-- 
Masami Hiramatsu (Google) [off-list ref]