Re: [PATCH 4/4] KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"

[Yan Zhao]

On Tue, Aug 11, 2026 at 08:25:32AM +0800, Yan Zhao wrote:
> On Thu, Aug 06, 2026 at 02:40:50PM -0700, Sean Christopherson wrote:
> > Harden the "map private PFN" flow against potentially-fatal bugs or future
> > KVM changes by checking for a stale "fault" prior to actually mapping the
> > PFN into the guest.  While it should be impossible for the "page fault" to
> > become stale, the sanity check is cheap, whereas a broken assumption would
> > have a high probability of leading to a guest-expoitable use-after-free.
> > 
> > Snapshot the invalidation sequence after acquiring mmu_lock to avoid false
> > positives, even though doing so completely voids anys and all protection
> > against unexpected invalidations.  Pretty much the entire point of
> > kvm_tdp_mmu_map_private_pfn() is that it allows mapping a PFN that was
> > gifted by the caller, i.e. the caller would have to mess up its one and
> > only responsibility.
> > 
> > Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx>
> > ---
> >  arch/x86/kvm/mmu/mmu.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> > index 379f570ef04f..76e3cd717324 100644
> > --- a/arch/x86/kvm/mmu/mmu.c
> > +++ b/arch/x86/kvm/mmu/mmu.c
> > @@ -5210,6 +5210,16 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
> >  		 */
> >  		WARN_ON_ONCE(kvm_test_request(KVM_REQ_MMU_FREE_OBSOLETE_ROOTS, vcpu));
> >  
> > +		/*
> > +		 * Snapshot the invalidation sequence counter after acquiring
> > +		 * mmu_lock, as guest_memfd guarantees the validity of the pfn,
> > +		 * i.e. any concurrent invalidations are guaranteed to be
> > +		 * irrelevant.
> > +		 */
> > +		fault.mmu_seq = vcpu->kvm->mmu_invalidate_seq;
> Could you explain more about the conditions under which a fault is stale while
> guest_memfd guarantees the validity of the pfn?
> 
> Given that kvm_tdp_mmu_map_private_pfn() already asserts holding slots_lock and
> invalidate_lock, I can't think of one. If we add the sanity check because it's
> cheap, why don't we save fault.mmu_seq before getting the pfn to guard against
> stale pfn as well?
> 
> > +		if (is_page_fault_stale(vcpu, &fault))
is_page_fault_stale() gets sp from vcpu->arch.mmu->root.hpa:
"struct kvm_mmu_page *sp = root_to_sp(vcpu->arch.mmu->root.hpa);".

For TDX, sp should be from vcpu->arch.mmu->mirror_root_hpa?

> > +			continue;
> > +
> >  		r = kvm_tdp_mmu_map(vcpu, &fault);
> >  	} while (r == RET_PF_RETRY);
> >