Re: [PATCH v4 17/18] KVM: guest_memfd: Align the gfn as well as the pfn when "preparing" a folio

[sashiko-bot@kernel.org]

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

New issues:
- [Medium] Aligning GFN down without validating memslot boundaries introduces host out-of-bounds physical memory access when preparing huge folios.

Pre-existing issues:
- [Critical] Concurrent execution of KVM_GMEM_POPULATE and vCPU page faults allows unprivileged userspace to trigger host hardware crashes and guest data corruption.
--

--- Patch [17]: [PATCH v4 17/18] KVM: guest_memfd: Align the gfn as well as the pfn when "preparing" a folio ---
commit 5debe95f8712f79807df583e8d5e927a97b4c012
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Align the gfn as well as the pfn when "preparing" a folio

When preparing a guest_memfd folio, this patch aligns the gfn as well as
the pfn to preserve the effective gfn+pfn pair. This prevents a mismatch
set and unintentional guest crashes due to assigning the pfn at the wrong
gfn for huge folios.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index cd7a795834cb6..4082cde3f3fca 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -87,8 +87,8 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	 * checked when creating memslots.
>  	 */
>  	WARN_ON_ONCE(!IS_ALIGNED(slot->gmem.pgoff, nr_pages));
> +	gfn = ALIGN_DOWN(gfn, nr_pages);
>  	index = kvm_gmem_get_index(slot, gfn);
> -	index = ALIGN_DOWN(index, nr_pages);

[Severity: Medium]
If a userspace caller creates a guest_memfd memslot with a base_gfn that is
not aligned to the huge folio size, will computing the index from the aligned
gfn result in a misaligned index?

When passed to kvm_arch_gmem_prepare(), couldn't this cause the iteration
to go past the end of the huge folio, resulting in an out-of-bounds host
physical memory access?

[Severity: Critical]
This is a pre-existing issue, but I noticed a potential race condition
between KVM_GMEM_POPULATE and vCPU page faults in this same file.

Looking at __kvm_gmem_populate(), the folio lock is dropped before calling
post_populate():

virt/kvm/guest_memfd.c:__kvm_gmem_populate() {
    ...
	folio_unlock(folio);
    ...
	ret = post_populate(kvm, gfn, pfn, src_page, opaque);
    ...
}

Could a concurrent vCPU page fault on the same GFN call kvm_gmem_get_pfn(),
acquire the folio lock, see that it is not uptodate, and clear the page
while the populate thread is executing post-populate updates like
KVM_SEV_SNP_LAUNCH_UPDATE?

virt/kvm/guest_memfd.c:kvm_gmem_get_pfn() {
    ...
	if (!folio_test_uptodate(folio)) {
		clear_highpage(folio_page(folio, 0));
    ...
}

Would this allow unprivileged userspace to trigger host hardware crashes
(Machine Check Exceptions) and guest data corruption simply by running a
vCPU thread concurrently with the population ioctl?

>  
>  	return kvm_arch_gmem_prepare(kvm, gfn_to_gpa(gfn),
>  				     folio_file_pfn(folio, index), nr_pages,

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