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

[Hyunwoo Kim <imv4bel@gmail.com>]

On Thu, Jun 11, 2026 at 05:47:52AM -0700, Sean Christopherson wrote:
> On Thu, Jun 11, 2026, Hyunwoo Kim wrote:
> > 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:
> > > > ---
> > > >  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.
> 
> I'm inclined to do nothing.  rmp_make_shared() should only fail in this case if
> there's a fatal bug somewhere, no?  Either that or do BUG_ON(), because at some
> point these types of errors are simply unrecoverable.

A guest can make a gmem page a VMSA via AP creation, and if that gfn is
then hole-punched, a page that is still assigned to the guest is
returned to the host in sev_gmem_invalidate(), which looked like it could
lead to a host RMP PF, so I sent the patch. Other sites such as
snp_page_reclaim() leak the page on this failure rather than freeing it.

If you think this isn't a real problem, leaving it as is seems fine to me.
I don't see a good place to put a BUG_ON.