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 Closes: https://lore.kernel.org/all/sashiko-outbox-191120@kernel.org Cc: Carlos López Signed-off-by: Sean Christopherson --- 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