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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Snapshotting `mmu_invalidate_seq` after acquiring `mmu_lock` and after PFN resolution violates the mandatory MMU retry protocol sequence, bypassing generation-safety checks against concurrent invalidations.
--

commit 3d8d7ec872ba8330e37f8fdcf1804dda38c1cf1e
Author: Sean Christopherson <seanjc@google.com>

KVM: x86/mmu: Add sanity check to detect stale page faults in "map private PFN"

This commit adds a check for stale page faults prior to mapping a private PFN
into the guest. It intentionally snapshots the invalidation sequence after
acquiring the MMU lock to avoid false positives.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 19a501029f089..79c450d677b40 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -5236,6 +5236,18 @@ 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;
> +		if (is_page_fault_stale(vcpu, &fault)) {

[Severity: High]
Does capturing mmu_invalidate_seq after acquiring mmu_lock and after PFN
resolution bypass the mandatory MMU retry protocol safety checks?

According to the KVM memory management guidelines, the invalidation sequence
should be captured before resolving the PFN, and the lock must be acquired
afterward. By snapshotting the sequence here and immediately checking it
via is_page_fault_stale(), the check effectively compares the counter to
itself, which guarantees it will pass.

Could this bypass allow the installation of stale translations or cause a
use-after-free if the caller's guarantees ever fail in the future?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826164214.756512-1-seanjc@google.com?part=4