Re: [PATCH v5 4/7] KVM: guest_memfd: Fold __kvm_gmem_prepare_folio() into its sole caller

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Refactoring `kvm_gmem_prepare_folio()` and `kvm_gmem_make_private()` to align `gfn` before computing `index` produces a mismatched and unaligned `pfn` if the memslot's `base_gfn` is not aligned to the huge folio size. This causes the architecture-specific prepare function to write beyond the bounds of the folio, leading to host physical memory corruption.

Pre-existing issues:
- [High] In SEV-SNP, if large RMP entry preparation fails for a huge folio, KVM will only prepare the base 4K page, causing an incomplete initialization and an infinite page fault loop for subpage accesses.
--

--- Patch [4]: [PATCH v5 4/7] KVM: guest_memfd: Fold __kvm_gmem_prepare_folio() into its sole caller ---
commit ef3a2643968d189a00f8b26552b9b814bf1aa548
Author: Sean Christopherson <seanjc@google.com>

KVM: guest_memfd: Fold __kvm_gmem_prepare_folio() into its sole caller

This commit folds __kvm_gmem_prepare_folio() into its caller, removing the
single-use tail-call helper. It also adjusts the alignment logic to align the
gfn before computing the index, and converts a WARN to WARN_ON_ONCE.

> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 28cdcf6007453..f065dedfbba8c 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c

[ ... ]

> @@ -97,11 +85,15 @@ static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
>  	 * The order will be passed when creating the guest_memfd, and
>  	 * checked when creating memslots.
>  	 */
> -	WARN_ON(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
> +	WARN_ON_ONCE(!IS_ALIGNED(slot->gmem.pgoff, folio_nr_pages(folio)));
> +	gfn = ALIGN_DOWN(gfn, folio_nr_pages(folio));
>  	index = kvm_gmem_get_index(slot, gfn);
> -	index = ALIGN_DOWN(index, folio_nr_pages(folio));
>  
> -	return __kvm_gmem_prepare_folio(kvm, slot, index, folio);
> +	return kvm_arch_gmem_prepare(kvm, gfn, folio_file_pfn(folio, index),
> +				     folio_order(folio));

[Severity: Critical]
Does aligning gfn before computing index cause an unaligned index when
slot->base_gfn is not huge-page aligned?

If a userspace process maps a guest_memfd using KVM_SET_USER_MEMORY_REGION2
with a base_gfn that is not aligned to the huge page size,
kvm_gmem_get_index() could return an unaligned index.

Because folio_file_pfn() applies the index offset to the base PFN:

virt/kvm/guest_memfd.c:folio_file_pfn() {
    return folio_pfn(folio) + (index & (folio_nr_pages(folio) - 1));
}

an unaligned index causes it to return an offset PFN instead of the
folio's base PFN.

Could this lead to host physical memory corruption? The architecture
backend processes the full page count starting from the offset PFN,
which would write beyond the allocated folio boundaries and overwrite
unrelated host physical memory.

It looks like this same alignment pattern is also present in
kvm_gmem_make_private().


[Severity: High]
This isn't a bug introduced by this patch, but could this generic
function incorrectly assume a successful return means the entire
huge folio was prepared?

Looking at sev_gmem_prepare() in arch/x86/kvm/svm/sev.c, if it falls
back to PG_LEVEL_4K for a huge folio:

    } else {
        level = PG_LEVEL_4K;
        pfn_aligned = pfn;
        gfn_aligned = gfn;
    }

    rc = rmp_make_private(pfn_aligned, gfn_to_gpa(gfn_aligned), level, sev->asid, false);

It appears rmp_make_private() is called only once for the base 4K page,
without looping over the remaining pages in the huge folio.

Since kvm_arch_gmem_prepare() then returns success to this generic code,
are the remaining 511 pages left uninitialized, potentially causing an
infinite page fault loop for subsequent subpage accesses?

> +#else
> +	return 0;
> +#endif
>  }

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