linux-kernel - Re: [RFC PATCH 07/12] KVM: TDX: Avoid a double-KVM_BUG_ON() in tdx_sept_zap_private

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aLD+CCAqDNlUklR8@yzhao56-desk.sh.intel.com>
Date: Fri, 29 Aug 2025 09:10:32 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
CC: "pbonzini@...hat.com" <pbonzini@...hat.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "Annapurve,
 Vishal" <vannapurve@...gle.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "michael.roth@....com"
	<michael.roth@....com>, "Weiny, Ira" <ira.weiny@...el.com>
Subject: Re: [RFC PATCH 07/12] KVM: TDX: Avoid a double-KVM_BUG_ON() in
 tdx_sept_zap_private_spte()

On Thu, Aug 28, 2025 at 10:50:06PM +0800, Edgecombe, Rick P wrote:
> On Wed, 2025-08-27 at 19:19 -0700, Rick Edgecombe wrote:
> > On Tue, 2025-08-26 at 17:05 -0700, Sean Christopherson wrote:
> > > Return -EIO immediately from tdx_sept_zap_private_spte() if the number of
> > > to-be-added pages underflows, so that the following "KVM_BUG_ON(err, kvm)"
> > > isn't also triggered.  Isolating the check from the "is premap error"
> > > if-statement will also allow adding a lockdep assertion that premap errors
> > > are encountered if and only if slots_lock is held.
> > > 
> > > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> > > ---
> > 
> > Reviewed-by: Rick Edgecombe <rick.p.edgecombe@...el.com>
> 
> There is actually another KVM_BUG_ON() in the path here:
> 
> static void remove_external_spte(struct kvm *kvm, gfn_t gfn, u64 old_spte,
> 				 int level)
> {
> 	kvm_pfn_t old_pfn = spte_to_pfn(old_spte);
> 	int ret;
> 
> 	/*
> 	 * External (TDX) SPTEs are limited to PG_LEVEL_4K, and external
> 	 * PTs are removed in a special order, involving free_external_spt().
> 	 * But remove_external_spte() will be called on non-leaf PTEs via
> 	 * __tdp_mmu_zap_root(), so avoid the error the former would return
> 	 * in this case.
> 	 */
> 	if (!is_last_spte(old_spte, level))
> 		return;
> 
> 	/* Zapping leaf spte is allowed only when write lock is held. */
> 	lockdep_assert_held_write(&kvm->mmu_lock);
> 	/* Because write lock is held, operation should success. */
> 	ret = kvm_x86_call(remove_external_spte)(kvm, gfn, level, old_pfn);
> ->	KVM_BUG_ON(ret, kvm);
> 
> We don't need to do it in this patch, but we could remove the return value in
> .remove_external_spte, and the KVM_BUG_ON(). Just let remove_external_spte
> handle it internally.
+1. Triggering KVM_BUG_ON() only in TDX internally is better.