linux-kernel - Re: [RFC PATCH 09/12] KVM: TDX: Fold tdx_mem_page_record_premap

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: <12b2b02b58b2262cf58f047a40c9af1d11bb09e7.camel@intel.com>
Date: Thu, 28 Aug 2025 23:17:17 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "seanjc@...gle.com" <seanjc@...gle.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com"
	<pbonzini@...hat.com>, "Annapurve, Vishal" <vannapurve@...gle.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Zhao, Yan Y"
	<yan.y.zhao@...el.com>, "michael.roth@....com" <michael.roth@....com>,
	"Weiny, Ira" <ira.weiny@...el.com>
Subject: Re: [RFC PATCH 09/12] KVM: TDX: Fold tdx_mem_page_record_premap_cnt()
 into its sole caller

On Thu, 2025-08-28 at 14:57 -0700, Sean Christopherson wrote:
> Agreed, and it would eliminate the need for a "flags" argument.  But keeping it
> in the mmu_lock critical section means KVM can WARN on failures.  If it's moved
> out, then zapping S-EPT entries could induce failure, and I don't think it's
> worth going through the effort to ensure it's impossible to trigger S-EPT removal.
> 
> Note, temoving S-EPT entries during initialization of the image isn't something
> I want to official support, rather it's an endless stream of whack-a-mole due to
> obsurce edge cases
> 
> Hmm, actually, maybe I take that back.  slots_lock prevents memslot updates,
> filemap_invalidate_lock() prevents guest_memfd updates, and mmu_notifier events
> shouldn't ever hit S-EPT.  I was worried about kvm_zap_gfn_range(), but the call
> from sev.c is obviously mutually exclusive, TDX disallows KVM_X86_QUIRK_IGNORE_GUEST_PAT
> so same goes for kvm_noncoherent_dma_assignment_start_or_stop, and
> 

Yea, in the other thread Yan was suggesting the same thing from the KVM side:
https://lore.kernel.org/all/aK%2Fsdr2OQqYv9DBZ@yzhao56-desk.sh.intel.com/

But was concerned about "Unexpected zaps" (kvm_zap_gfn_range()). I think maybe
we could think about KVM_BUG_ON() in the case of mirror EPT to cover it from
another angle. IIRC we discussed this at some point.

I was wondering about TDH.MR.EXTEND error conditions. Coming back now, I'm not
sure what I was thinking.

> while I'm 99% certain there's a way to trip __kvm_set_or_clear_apicv_inhibit(),
> the APIC page has its own non-guest_memfd memslot and so can't be used for the
> initial image, which means that too is mutually exclusive.

Hmm, well maybe KVM_BUG_ON() for kvm_zap_gfn_range() only if this gets
addressed.

> 
> So yeah, let's give it a shot.  Worst case scenario we're wrong and TDH_MR_EXTEND
> errors can be triggered by userspace.
> 
> > But maybe a better reason is that we could better handle errors
> > outside the fault. (i.e. no 5 line comment about why not to return an error in
> > tdx_mem_page_add() due to code in another file).
> > 
> > I wonder if Yan can give an analysis of any zapping races if we do that.
> 
> As above, I think we're good?

Works for me.