Re: [PATCH v2 3/4] KVM: SEV: Drop page refcount early in VMSA reload
Ackerley Tng <[email protected]>
| Newsgroups | dev.linux.lists.kvmarm,dev.linux.lists.sashiko-reviews,org.kernel.vger.kvm |
|---|---|
| Message-ID | <CAEvNRgH5ZQPGYzg+YdtEktZ72DgK9PBbzcmC_QFN6M0_6wuo1w@mail.gmail.com> |
Sean Christopherson <[email protected]> writes: > On Fri, Aug 21, 2026, Ackerley Tng wrote: >> Ackerley Tng <[email protected]> writes: >> >> This is a pre-existing issue, but can this allow a malicious guest to >> >> cause a host kernel panic? >> >> >> >> Looking at sev_gmem_make_shared() in arch/x86/kvm/svm/sev.c: >> >> >> >> arch/x86/kvm/svm/sev.c:sev_gmem_make_shared() { >> >> ... >> >> 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)) >> >> goto next_pfn; >> >> ... >> >> } >> >> >> >> If an untrusted SEV-SNP guest modifies a guest_memfd page state to VMSA >> >> using RMPADJUST, and the page is subsequently freed, rmp_make_shared() >> >> will architecturally fail on the VMSA page. >> > >> > I need some help with this one. >> > >> > For the initial VMSA page, IIUC snp_safe_alloc_page() gets a >> > non-guest_memfd page. This won't ever go through sev_gmem_make_shared(), >> > so we're good. snp_leak_pages() doesn't actually leak pages, it just >> > tracks leaked pages, but __free_page() is skipped so the page is really >> > leaked. >> > >> >> Because the error is bypassed without calling snp_leak_pages(), the >> >> firmware-owned page is returned to the host page allocator. >> > >> > For guest-provided VMSA pages, those can be guest_memfd pages. The >> > refcount taken is the one guest_memfd holds. Without considering >> > conversions, just considering truncation, sev_gmem_make_shared() is >> > called from kvm_gmem_free_folio(). Sashiko is saying that >> > rmp_make_shared() will fail, and the folio is freed. >> > >> > Sounds like this would be a problem if the folio is used elsewhere...? >> >> I looked closer, "15.36.12 Running SNP-Active Virtual Machines" in APM >> Vol2 says "After a successful VMRUN, the VMCB page, as well as any AVIC >> Backing Page and VMSA Page are marked as in-use by hardware, and any >> attempt to modify the RMP entries for these pages via instructions like >> RMPUPDATE will result in a FAIL_INUSE response. The in-use marking is >> automatically cleared by hardware after a #VMEXIT event". >> >> So it's not true that rmp_make_shared() will always fail on a VMSA page, >> it's only true if the guest vCPU is running. > > Correct. > >> RMPADJUST sets up the VMSA page, and telling the host using >> SVM_VMGEXIT_AP_CREATE is how the VMSA is set up to be usable. Then, the >> other vCPU needs to be started and running, and then: >> >> If the host truncates the VMSA page or host closes the guest_memfd file >> containing some other vCPU's VMSA page, sev_gmem_make_shared() on the >> VMSA page should fail, but the page is released anyway and we have a >> problem. >> 74bdfb2f6d18 ("KVM: SEV: Forcefully invalidate SNP VMSA if its backing gmem page is zapped") fixes this :) I didn't follow that full discussion and now I get it. Here's an explanation from another angle: This VMSA invalidation is interesting in that there isn't really any communication to SNP, because the host doesn't really have a direct way undo the RMPADJUST. What the host can do is to RMPUPDATE the entry to be shared, which is what truncation already does. The key thing is that the RMPUPDATE has to be done while the vCPU isn't running. Other than invalidating whatever kvm is tracking in svm->sev_es.snp_guest_vmsa_gpa and in svm->vmcb->control.vmsa_pa, sev_gmem_invalidate_range() has another side effect, which is that if there is an ongoing invalidation in another CPU, it keeps requesting KVM_REQ_VMSA_PAGE_RELOAD, which stops the vCPU from entering the guest. Keeping the vCPU out of the guest is the thing that allows the RMPUPDATE (rmp_make_shared()) to work. This loop keeps the vCPU out of the guest. It's kicked off by kvm_arch_gmem_invalidate_range(), and loops until kvm_gmem_invalidate_end() is called. .free_folio, which does the RMPUPDATE, is called synchronously between kvm_gmem_invalidate_start() and kvm_gmem_invalidate_end(), and so we can be sure the RMPUPDATE will not happen while the vCPU is running. A guest-provided VMSA is set up for a given vCPU (identified using apic_id) at a lower VMPL. I was also wondering: what if a guest just decides to _not_ report SVM_VMGEXIT_AP_CREATION? Is it possible to just RMPADJUST and not tell the host, such that sev_gmem_invalidate_range() doesn't know about the donated VMSA? From working on the reproducer I don't think this is possible, since without informing the host, there's no way for the guest to use the vCPU with the RMPADJUST-ed VMSA. If there's no way to run that vCPU, then guest_memfd is always free to RMPUPDATE to make that page shared. The host can always RMPUPDATE a VMSA page as long as the VMSA page is not in use. >> An unsolicited shared->private conversion is fine since the VMSA page is >> already private. >> I need to look at conversions again to take this into account, will explain more over on that series. Meanwhile, there's no issue here because rmp_make_shared() will succeed, so the leak won't happen. >> sev_gmem_make_shared() would fail on an unsolicited private->shared >> conversion. The RMP entry would still indicate private, and guest_memfd >> would track it as shared...? Bad. >> >> I've been trying to write a mini reproducer like a selftest but I've >> still not gotten it to work yet. If someone knows there's no issue here >> please let me know! > > I have a reproducer I created to prove and validate 74bdfb2f6d18 ("KVM: SEV: > Forcefully invalidate SNP VMSA if its backing gmem page is zapped"), I'll make > sure to run it when applying (the final version of) this series. I got the reproducer working :). I'm guessing sharing the reproducer publicly isn't allowed so I won't.