mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: "Masami Hiramatsu (Google)" <mhiramat@kernel.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Peter Zijlstra <peterz@infradead.org>,
	 Ingo Molnar <mingo@kernel.org>,
	x86@kernel.org, Jinchao Wang <wangjinchao600@gmail.com>,
	 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	 Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	 "H . Peter Anvin" <hpa@zytor.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	 Ian Rogers <irogers@google.com>,
	linux-kernel@vger.kernel.org,
	 linux-trace-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	 linux-perf-users@vger.kernel.org,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org
Subject: Re: [PATCH v16 02/13] KVM: x86: Prevent host DR7 debug register leak into guest OS on NMI
Date: Mon, 14 Sep 2026 07:35:48 -0700	[thread overview]
Message-ID: <aqgGRKOE138ePqmX@google.com> (raw)
In-Reply-To: <178939019982.94750.4441254006258061798.stgit@devnote2>

On Mon, Sep 14, 2026, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> When KVM enters a guest OS, host hardware breakpoints are disabled
> before running the guest. However, an NMI can occur during guest
> execution, where local_db_save() or arch_install_hw_breakpoint()
> can be invoked.
> 
> In particular, if local_db_save() or arch_install_hw_breakpoint()
> is executed from NMI, hardware DR7 can be modified or restored with
> host breakpoint settings, leaking host breakpoints into the guest OS
> or clobbering the guest's debug registers.

Not for local_db_save(), at least not AFAICT.  On VM-Exit, both Intel and AMD
purge DR7, i.e. load 0x400, so local_db_save() => local_db_restore() is more or
less a nop.  Even if that weren't the case, actually saving/restoring DR7 would
be the right thing to do, in any context.

> Introduce a per-CPU flag, cpu_dr_in_guest, to indicate that the CPU
> is executing in guest mode. Set this flag in vcpu_enter_guest()
> during entering the guest with disabling host breakpoints.
> If this flag is set, local_db_save() and local_db_restore() return
> immediately, and arch_install_hw_breakpoint() returns an error.
> In addition, protect cpu_dr_in_guest in within_cpu_entry() to
> prevent recursive #DB exceptions.
> 
> Fixes: f85d40160691 ("KVM: X86: Disable hardware breakpoints unconditionally before kvm_x86->run()")
> Assisted-by: Antigravity:gemini-3.8-flash
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> Changes in v16:
>  - Newly added.
> ---
>  arch/x86/include/asm/debugreg.h |    6 ++++++
>  arch/x86/kernel/hw_breakpoint.c |   10 ++++++++++
>  arch/x86/kvm/x86.c              |    7 +++++++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/arch/x86/include/asm/debugreg.h b/arch/x86/include/asm/debugreg.h
> index 854d82b88ff4..50f830972698 100644
> --- a/arch/x86/include/asm/debugreg.h
> +++ b/arch/x86/include/asm/debugreg.h
> @@ -18,6 +18,7 @@
>  #define DR7_FIXED_1	0x00000400
>  
>  DECLARE_PER_CPU(unsigned long, cpu_dr7);
> +DECLARE_PER_CPU(bool, cpu_dr_in_guest);
>  
>  #ifndef CONFIG_PARAVIRT_XXL
>  /*
> @@ -129,6 +130,9 @@ static __always_inline unsigned long local_db_save(void)
>  {
>  	unsigned long dr7;
>  
> +	if (this_cpu_read(cpu_dr_in_guest))
> +		return 0;

This is broken.  If an NMI hits between KVM writing cpu_dr_in_guest and clearing
DR7, and there are active breakpoints, then local_db_save() won't disable breakpoints
as it should, and the relevant code in exc_nmi() will run with breakpoints enabled.

	kvm_load_xfeatures(vcpu, true);

	this_cpu_write(cpu_dr_in_guest, true);
	barrier();

  <NMI here is problematic>

	if (unlikely(vcpu->arch.switch_db_regs &&
		     !(vcpu->arch.switch_db_regs & KVM_DEBUGREG_AUTO_SWITCH))) {
		set_debugreg(DR7_FIXED_1, 7);
		set_debugreg(vcpu->arch.eff_db[0], 0);

> +
>  	if (cpu_feature_enabled(X86_FEATURE_HYPERVISOR) && !hw_breakpoint_active())
>  		return 0;
>  
> @@ -157,6 +161,8 @@ static __always_inline void local_db_restore(unsigned long dr7)
>  	 * not be good.
>  	 */
>  	barrier();
> +	if (this_cpu_read(cpu_dr_in_guest))
> +		return;
>  	if (dr7)
>  		set_debugreg(dr7, 7);
>  }
> diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> index f846c15f21ca..68de7ed79d88 100644
> --- a/arch/x86/kernel/hw_breakpoint.c
> +++ b/arch/x86/kernel/hw_breakpoint.c
> @@ -40,6 +40,9 @@
>  DEFINE_PER_CPU(unsigned long, cpu_dr7);
>  EXPORT_PER_CPU_SYMBOL(cpu_dr7);
>  
> +DEFINE_PER_CPU(bool, cpu_dr_in_guest);
> +EXPORT_PER_CPU_SYMBOL_GPL(cpu_dr_in_guest);
> +
>  /* Per cpu debug address registers values */
>  static DEFINE_PER_CPU(unsigned long, cpu_debugreg[HBP_NUM]);
>  
> @@ -102,6 +105,9 @@ int arch_install_hw_breakpoint(struct perf_event *bp)
>  
>  	lockdep_assert_irqs_disabled();
>  
> +	if (this_cpu_read(cpu_dr_in_guest))
> +		return -EBUSY;

If we decide this is how to fix arch_install_hw_breakpoint() clobbering DRs from
NMI context, I would rather have more generic flag to tell perf that KVM is about
to enter the guest, e.g. so that we don't have to separately solve the same problem
for other perf events:

https://lore.kernel.org/all/3585d823-00f3-46ae-a799-b62a95743e76@linux.intel.com

  reply	other threads:[~2026-09-14 14:35 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-14 12:49 [PATCH v16 00/13] tracing: wprobe: x86: Add wprobe for watchpoint Masami Hiramatsu (Google)
2026-09-14 12:49 ` [PATCH v16 01/13] x86/mce: Fix hardware debug register corruption on task migration Masami Hiramatsu (Google)
2026-09-14 12:49 ` [PATCH v16 02/13] KVM: x86: Prevent host DR7 debug register leak into guest OS on NMI Masami Hiramatsu (Google)
2026-09-14 14:35   ` Sean Christopherson [this message]
2026-09-16 23:45     ` Masami Hiramatsu
2026-09-14 12:50 ` [PATCH v16 03/13] x86/hw_breakpoints: Make DR7 updates NMI safe Masami Hiramatsu (Google)
2026-09-14 12:50 ` [PATCH v16 04/13] x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-09-14 12:50 ` [PATCH v16 05/13] HWBP: Add modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-09-14 12:50 ` [PATCH v16 06/13] tracing/wprobe: Add wprobe (watchpoint probe) trace event support Masami Hiramatsu (Google)
2026-09-14 12:50 ` [PATCH v16 07/13] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires Masami Hiramatsu (Google)
2026-09-14 12:51 ` [PATCH v16 08/13] selftests: tracing: Add a basic testcase for wprobe Masami Hiramatsu (Google)
2026-09-14 12:51 ` [PATCH v16 09/13] selftests: tracing: Add syntax " Masami Hiramatsu (Google)
2026-09-14 12:51 ` [PATCH v16 10/13] tracing/wprobe: Add set_wprobe and clear_wprobe event triggers Masami Hiramatsu (Google)
2026-09-14 12:51 ` [PATCH v16 11/13] selftests: tracing: Add wprobe trigger testcase Masami Hiramatsu (Google)
2026-09-14 12:51 ` [PATCH v16 12/13] tracing/wprobe: Support BTF typecast in fetchargs Masami Hiramatsu (Google)
2026-09-14 12:52 ` [PATCH v16 13/13] tracing/wprobe: Support BTF struct offset resolution in set_wprobe trigger Masami Hiramatsu (Google)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqgGRKOE138ePqmX@google.com \
    --to=seanjc@google.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wangjinchao600@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®