Re: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Dropping the page refcount early introduces a race condition where a concurrent `guest_memfd` invalidation can trigger a false-positive 'Unexpected RMP fault' kernel warning.
--

--- Patch [2]: [PATCH v2 2/4] KVM: SEV: Drop page refcount early during RMP fault handling ---
commit 7d0f356d8592ce1804052b0fdb61fccf3a11ecaa
Author: Sean Christopherson <[email protected]>

KVM: SEV: Drop page refcount early during RMP fault handling

When handling an RMP fault, KVM retrieves the PFN for a private GPA from
guest_memfd. This patch drops the page reference immediately after retrieving
the PFN instead of holding it across the entire handler, and adopts the MMU
invalidation protocol to guard committing anything based on the PFN.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b2738362a928b..b34b11d7f8fad 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c

[ ... ]

> @@ -5030,18 +5031,22 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  		return;
>  	}
>  
> +	mmu_seq = kvm->mmu_invalidate_seq;
> +	smp_rmb();
> +
>  	ret = kvm_gmem_get_pfn(kvm, slot, gfn, &pfn, &page, &order);
>  	if (ret) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no backing page for private GPA 0x%llx\n",
>  				    gpa);
>  		return;
>  	}
> +	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);
>  	if (ret || !assigned) {
>  		pr_warn_ratelimited("SEV: Unexpected RMP fault, no assigned RMP entry found for GPA 0x%llx PFN 0x%llx error %d\n",
>  				    gpa, pfn, ret);
> -		goto out_no_trace;
> +		return;
>  	}

[Severity: Medium]
Since the page reference is dropped before this lookup in
sev_handle_rmp_fault(), could a concurrent guest_memfd hole-punch drop
the file's reference to the page and free it?

If the page is freed, the RMP entry would transition to shared/unassigned.
Could this then cause snp_lookup_rmpentry() to see the page as unassigned
and incorrectly emit this warning before the mmu_invalidate_retry_gfn()
check happens?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2