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

Yan Zhao <[email protected]>
Newsgroups org.kernel.vger.kvm,org.kernel.vger.linux-kernel
Message-ID <anpr/[email protected]>
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 <[email protected]>
> ---
>  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))
> +			continue;
> +
>  		r = kvm_tdp_mmu_map(vcpu, &fault);
>  	} while (r == RET_PF_RETRY);
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.