From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A267D472521; Wed, 16 Sep 2026 23:45:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789602333; cv=none; b=SALJWOO0q/j5/HEs+iYtFXwRlw0ByrHKnazVq6tBpSsL0769PTX2ZTXRiML/9h+O6zy20xKsWUD6QSJoEgpfEgL4K6VM7nO7mYgJg+KQ4fbJClsJaKfCestk2hR/r7VoTPxK8y8o29yb3rdwcmiwnng4ZhfwoaOwa1/mTx33o1Y= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1789602333; c=relaxed/simple; bh=Kh3rKS1+K7TDXnwpcYYaSVeBFbOeWE/01LMZJFeCs8k=; h=Date:From:To:Cc:Subject:Message-Id:In-Reply-To:References: Mime-Version:Content-Type; b=I3tzTN5EkEL2uAg7O5FKFbtvaKNajTQfNfMG70Zcke3R//OtbqX8ESgPzlAS78oxd4y3pIsHE90m+E5+UbEut+umlETOVEzh0sSfZ2AmSBAf6I3BApVoUtGDsHNRusIoVEBAy1wijaBYUErUmNKBvXjVc7E+Ktk2jWjLxxoQSC0= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HDmAf4+L; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="HDmAf4+L" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E282B1F000FF; Wed, 16 Sep 2026 23:45:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1789602332; bh=Uom+5GiRwnUi0P13tTGi61WOHauRp/h8S/b2KqC1Afk=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=HDmAf4+LvbmJSAoQRmaGZ2c2JeChzAbl08VlEepE6bI4BXd4YKpP0Ukb9lhdtMtTF HNUofo1oxUREMHfcP9KNAjrKP0QQlkeXuYmtD00rDEaFt9fHDKV9BsjZU76LdwXvfg la9tvag7dT8NkJOmmq0wMp1pWwz4jAtu708Bj05Kv6dYm8XdJevRGR5gyUg5t9oWwa Aeo8GeMCBIbpx3lCBZKZ8/1HDgBMuCW8G1sL9LbZ9IE0qw29fhud7UzGmUtDH8Ly20 kkh2QcnFg9fsfnmEoImihNReVklcb4lNcN9crUYuyiOyFWHwyAq1J0pCyLvYIDSZ4I Zx0w60SifExYQ== Date: Thu, 17 Sep 2026 08:45:26 +0900 From: Masami Hiramatsu (Google) To: Sean Christopherson Cc: Steven Rostedt , Peter Zijlstra , Ingo Molnar , x86@kernel.org, Jinchao Wang , Mathieu Desnoyers , Thomas Gleixner , Borislav Petkov , Dave Hansen , "H . Peter Anvin" , Alexander Shishkin , Ian Rogers , linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-perf-users@vger.kernel.org, Paolo Bonzini , kvm@vger.kernel.org Subject: Re: [PATCH v16 02/13] KVM: x86: Prevent host DR7 debug register leak into guest OS on NMI Message-Id: <20260917084526.18128cef47424990d42bb6ce@kernel.org> In-Reply-To: References: <178939017565.94750.9431053336761330458.stgit@devnote2> <178939019982.94750.4441254006258061798.stgit@devnote2> X-Mailer: Sylpheed 3.8.0beta1 (GTK+ 2.24.33; x86_64-pc-linux-gnu) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Hi Sean, Thanks for your review! On Mon, 14 Sep 2026 07:35:48 -0700 Sean Christopherson wrote: > On Mon, Sep 14, 2026, Masami Hiramatsu (Google) wrote: > > From: Masami Hiramatsu (Google) > > > > 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. OK. > > > 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) > > --- > > 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(); > > > > 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); Oops, indeed! OK, so local_db_save/restore will just work as it is, but modifying DR7 should directly be prohibited. > > > + > > 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 OK, so introducing a new (someting like) cpu_in_trans_guest flag and check it from all affected places? Thank you, -- Masami Hiramatsu (Google)