Re: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent execution of KVM_SEV_SNP_LAUNCH_UPDATE and a vCPU page fault triggers a race condition leading to a host kernel panic.
--

--- Patch [4]: [PATCH v2 4/4] KVM: guest_memfd: Stop returning struct page from PFN lookup ---
commit 766989bc1910148a83bffa481ae22b1d1eb71f01
Author: Sean Christopherson <[email protected]>

KVM: guest_memfd: Stop returning struct page from PFN lookup

This commit stops returning a refcounted struct page from guest_memfd PFN
lookups, dropping the folio reference immediately instead. This prevents
concurrent guest page faults from holding elevated refcounts that would
interfere with in-place memory conversions between shared and private states.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index b596486d184ca..589762140c3ef 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -780,11 +779,7 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>  #endif
>  
>  	folio_unlock(folio);
> -
> -	if (!r)
> -		*page = folio_file_page(folio, index);
> -	else
> -		folio_put(folio);
> +	folio_put(folio);
>  
>  	return r;
>  }

[Severity: Critical]
This is a pre-existing issue, but is it possible for a concurrent page
fault to trigger a host kernel panic via a hardware RMP fault here?

Looking at kvm_gmem_get_pfn() in virt/kvm/guest_memfd.c, if the folio is
not up to date, it zeroes the page and marks it up to date:

    if (!folio_test_uptodate(folio)) {
        clear_highpage(folio_page(folio, 0));
        folio_mark_uptodate(folio);
    }

However, in the KVM_SEV_SNP_LAUNCH_UPDATE path, __kvm_gmem_populate()
fetches a folio, unlocks it, and calls post_populate() (which maps to
sev_gmem_post_populate() and calls rmp_make_private()) before finally
marking the folio up to date.

If a malicious userspace process races a vCPU page fault against
KVM_SEV_SNP_LAUNCH_UPDATE, could the vCPU thread running kvm_gmem_get_pfn()
acquire the lock on the not-yet-uptodate folio and call clear_highpage()
after rmp_make_private() has already executed?

If clear_highpage() writes to the page after it has been made RMP-private,
this would trigger a fatal hardware RMP fault on the host CPU. If it writes
just before, it appears it could silently corrupt the encrypted firmware
payload.

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