On Sat, 22 Aug 2026 09:41:06 +0000
[email protected] wrote:
> > -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.)
>
> [ ... ]
> > 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
> [ ... ]
> > @@ -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) <[email protected]>
