Re: [PATCH 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:
- [Critical] Dropping the page reference early exposes the physical frame (PFN) to concurrent freeing and reallocation before hardware RMP operations (PSMASH) execute, leading to cross-VM memory corruption.
--

--- Patch [2]: [PATCH 2/4] KVM: SEV: Drop page refcount early during RMP fault handling ---
commit b722a12d672bd9f9b3789be73ead1445ff086955
Author: Ackerley Tng <[email protected]>

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

This patch modifies the RMP fault handler to drop the page reference retrieved
from guest_memfd early. The intention is to avoid holding the reference
throughout the rest of the handler, as the subsequent hardware operations and
page table zapping only require the physical frame number.

> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index b2738362a928b..19c31fa944e34 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
[ ... ]
> @@ -5036,12 +5036,13 @@ void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code)
>  				    gpa);
>  		return;
>  	}
> +	kvm_release_page_unused(page);
>  
>  	ret = snp_lookup_rmpentry(pfn, &assigned, &rmp_level);

[Severity: Critical]
Does dropping this page reference early expose the physical frame to concurrent
freeing and reallocation before the hardware operations execute?

Since sev_handle_rmp_fault() does not hold kvm->mmu_lock, if the page is
unpinned here, couldn't a concurrent fallocate(FALLOC_FL_PUNCH_HOLE) on the
guest_memfd unmap and free the page back to the host?

If the host then reallocates this physical page to a different VM as a
private 2MB hugepage, its reverse map entry would be assigned to the new VM.

When this vCPU resumes, it could call snp_lookup_rmpentry(), see the page is
assigned and 2MB, and proceed to execute snp_rmptable_psmash():

    if (rmp_level == PG_LEVEL_4K)
        goto out;

    ret = snp_rmptable_psmash(pfn);

Could this blindly shatter the other VM's 2MB entry into 4KB entries and
break hardware memory isolation?

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