[PATCH] KVM: VMX: Re-pend KVM_REQ_APIC_PAGE_RELOAD if getting backing pfn fails

[PATCH] KVM: VMX: Re-pend KVM_REQ_APIC_PAGE_RELOAD if getting backing pfn fails

[Sean Christopherson]

Retry loading the APIC-access page if getting the backing pfn fails for
any reason and APICv is enabled, i.e. if the vCPU will consume the value
stored in vmcs.APIC_ACCESS_ADDR.  Dropping the failed reload would result
in KVM running the vCPU with a stale backing pfn and in all likelihood
break APIC emulation/virtualization.  E.g. if KVM's page tables are able
to map the page, then guest accesses would go to the new physical page
instead of being treated as APIC accesses.

Opportunistically WARN if the memslot lookup fails, as KVM fully controls
the memslot, and VMX doesn't support inhibiting the APIC-access page, i.e.
KVM VMX should never delete or invalidate the memslot while the VM is alive
and reachable.

Do *not* WARN if getting the pfn fails, because despite KVM controlling the
memslot and allocating the backing page, there is nothing special about the
underlying VMA.  I.e. userspace can munmap(), mprotect(), etc. the VMA and
thus prevent KVM from faulting in the page.  If that happens, it's firmly
userspace's responsibility to re-install a valid mapping, e.g. to prevent
the vCPU from getting stuck in an endless KVM_REQ_APIC_PAGE_RELOAD loop.

Fixes: e8fd5e9e9984 ("kvm: fix crash in kvm_vcpu_reload_apic_access_page")
Reported-by: Sashiko Bot <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/sashiko-outbox-191120@kernel.org
Cc: Carlos López <clopez@suse.de>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---

FWIW, I actually verified this is a real bug, and that the fix works, by
hacking KVM to map the APIC-access page at a hardcoded virtual address, and
then modifying the xapic_ipi_test to unmap() and mmap() that virtual address
(usually at a different pfn).  I didn't Cc stable@ because unmapping the
APIC-access page is all but guaranteed to be fatal, e.g. the VMM would have
to blindly retry on -EFAULT when trying to fault-in the page into the MMU
inevitably fails.

 arch/x86/kvm/vmx/vmx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index b341af4c6057..13dcf9c12c83 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6923,8 +6923,10 @@ void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
 	 * APICv is enabled, but paranoia won't hurt in this case.
 	 */
 	slot = id_to_memslot(slots, APIC_ACCESS_PAGE_PRIVATE_MEMSLOT);
-	if (!slot || slot->flags & KVM_MEMSLOT_INVALID)
+	if (WARN_ON_ONCE(!slot || slot->flags & KVM_MEMSLOT_INVALID)) {
+		kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
 		return;
+	}
 
 	/*
 	 * Ensure that the mmu_notifier sequence count is read before KVM
@@ -6941,8 +6943,10 @@ void vmx_set_apic_access_page_addr(struct kvm_vcpu *vcpu)
 	 * if APICv is permanently inhibited, i.e. the memslot won't reappear.
 	 */
 	pfn = __kvm_faultin_pfn(slot, gfn, FOLL_WRITE, &writable, &refcounted_page);
-	if (is_error_noslot_pfn(pfn))
+	if (is_error_noslot_pfn(pfn)) {
+		kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu);
 		return;
+	}
 
 	read_lock(&vcpu->kvm->mmu_lock);
 	if (mmu_invalidate_retry_gfn(kvm, mmu_seq, gfn))

base-commit: d21eef2084a9af87ac2a783bc7c2b175ba114e4e
-- 
2.56.0.567.gdb80ae8cd5-goog