Re: [PATCH] KVM: SEV: Don't return a still-assigned gmem page to the host

[Hyunwoo Kim <imv4bel@gmail.com>]

On Wed, Jun 10, 2026 at 05:16:57PM -0500, Michael Roth wrote:
> On Thu, Jun 11, 2026 at 01:10:03AM +0900, Hyunwoo Kim wrote:
> > [You don't often get email from imv4bel@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> > 
> > sev_gmem_invalidate() is called when guest_memfd frees a gmem page.
> > For each PFN that is still assigned to the guest in the RMP table, it
> > transitions the page back to hypervisor-owned via rmp_make_shared()
> > before the page is returned to the host.
> > 
> > A guest-assigned page can reach this path while still private,
> > because the free path does not transition it beforehand and
> > sev_gmem_invalidate() is the only place that does. A gmem page used
> > as a vCPU's VMSA after SEV-SNP AP creation is one such case. When
> > rmp_make_shared() fails, the RMP entry remains guest-owned and the
> > host cannot use the page because of RMP protection, so it must not be
> > returned to the host. The existing code only issues WARN_ONCE() and
> > continues to the next PFN, returning the page to the host allocator.
> > 
> > Leak the page instead of freeing it, as kvm_rmp_make_shared(),
> > snp_page_reclaim() and sev_free_vcpu() already do when a transition
> > back to shared fails. snp_leak_pages() does not take a reference of
> > its own, and on this path the page is freed right after the hook
> > returns, so take a reference with folio_get() first to keep the page
> > from being freed.
> > 
> > Fixes: 8eb01900b018 ("KVM: SEV: Implement gmem hook for invalidating private pages")
> > Signed-off-by: Hyunwoo Kim <imv4bel@gmail.com>
> > ---
> >  arch/x86/kvm/svm/sev.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index 6c6a6d663e29..8fee6ec529f9 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -5178,8 +5178,12 @@ void sev_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end)
> > 
> >                 rc = rmp_make_shared(pfn, use_2m_update ? PG_LEVEL_2M : PG_LEVEL_4K);
> >                 if (WARN_ONCE(rc, "SEV: Failed to update RMP entry for PFN 0x%llx error %d\n",
> > -                             pfn, rc))
> > +                             pfn, rc)) {
> > +                       /* Still assigned to the guest; pin and leak rather than freeing. */
> > +                       folio_get(page_folio(pfn_to_page(pfn)));
> > +                       snp_leak_pages(pfn, use_2m_update ? PTRS_PER_PMD : 1);
> >                         goto next_pfn;
> > +               }
> 
> This roughly aligns with what would happen if snp_page_reclaim() fails
> in sev_gmem_post_populate(), while the guest is being initialized via
> KVM_SEV_SNP_LAUNCH_UPDATE ioctl, which calls into kvm_gmem_populate().
> 
> However, in kvm_gmem_populate(), we still free the page. Maybe, to
> address both cases, we should just add a parameter to snp_leak_pages()
> to tell it to take an extra ref and use that in both of these paths.
> 
> Or we can just do the direct folio_get() in both cases, the above
> formalizes the handling convention a little better though IMO.

If I understand correctly, an extra ref alone still seems to leave the
LRU corruption that sashiko flagged:

https://lore.kernel.org/all/20260610162623.061BA1F00898@smtp.kernel.org/

A gmem folio is on the unevictable LRU, and taking a ref keeps the folio
on the LRU. page->buddy_list, which snp_leak_pages() uses, shares the
same union as folio->lru, so leaking the page overwrites the folio's LRU
pointers. Both paths deal with a gmem folio, so the same applies.

To handle this properly, the folio would need to be taken off the LRU
before leaking, with something like folio_isolate_lru(), but that is
mm-internal and does not look usable from KVM. How should we proceed?
Please let me know if I am missing something.


Best regards,
Hyunwoo Kim