[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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] KVM: VMX: stale no-retry comment in vmx_set_apic_access_page_addr()
--

commit 635b83243ce9cf39c9f2bbd0198d7730dbb21505
Author: Sean Christopherson <seanjc@google.com>

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

This commit re-pends KVM_REQ_APIC_PAGE_RELOAD in
vmx_set_apic_access_page_addr() when looking up the APIC-access page memslot
or faulting in its backing pfn fails, preventing the vCPU from running with a
stale vmcs.APIC_ACCESS_ADDR. It also adds a WARN_ON_ONCE() if the private
memslot is missing or invalid.

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index b341af4c6057a..13dcf9c12c83a 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;
> +	}

[Severity: Low]
This isn't a bug, but in vmx_set_apic_access_page_addr() in
arch/x86/kvm/vmx/vmx.c, the full comment immediately above
__kvm_faultin_pfn() still states:

	/*
	 * No need to retry if the memslot does not exist or is invalid.  KVM
	 * controls the APIC-access page memslot, and only deletes the memslot
	 * if APICv is permanently inhibited, i.e. the memslot won't reappear.
	 */

Since both the invalid-memslot check and the is_error_noslot_pfn(pfn) check
now retry by calling kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu), and
VMX does not delete or inhibit the APIC-access page memslot while the VM is
alive, should this comment be updated or removed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105181742.3718448-1-seanjc@google.com?part=1

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

[Sean Christopherson]

On Thu, Nov 05, 2026, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] KVM: VMX: stale no-retry comment in vmx_set_apic_access_page_addr()
> --
> 
> commit 635b83243ce9cf39c9f2bbd0198d7730dbb21505
> Author: Sean Christopherson <seanjc@google.com>
> 
> KVM: VMX: Re-pend KVM_REQ_APIC_PAGE_RELOAD if getting backing pfn fails
> 
> This commit re-pends KVM_REQ_APIC_PAGE_RELOAD in
> vmx_set_apic_access_page_addr() when looking up the APIC-access page memslot
> or faulting in its backing pfn fails, preventing the vCPU from running with a
> stale vmcs.APIC_ACCESS_ADDR. It also adds a WARN_ON_ONCE() if the private
> memslot is missing or invalid.
> 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index b341af4c6057a..13dcf9c12c83a 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;
> > +	}
> 
> [Severity: Low]
> This isn't a bug, but in vmx_set_apic_access_page_addr() in
> arch/x86/kvm/vmx/vmx.c, the full comment immediately above
> __kvm_faultin_pfn() still states:
> 
> 	/*
> 	 * No need to retry if the memslot does not exist or is invalid.  KVM
> 	 * controls the APIC-access page memslot, and only deletes the memslot
> 	 * if APICv is permanently inhibited, i.e. the memslot won't reappear.
> 	 */
> 
> Since both the invalid-memslot check and the is_error_noslot_pfn(pfn) check
> now retry by calling kvm_make_request(KVM_REQ_APIC_PAGE_RELOAD, vcpu), and
> VMX does not delete or inhibit the APIC-access page memslot while the VM is
> alive, should this comment be updated or removed?

Gah, yeah, I'll fixup to this when applying:


	/*
	 * Retry if faulting in the page fails (or the memslot is unexpectedly
	 * invalid) even though KVM controls the APIC-access page memslot, and
	 * never deletes memslot for a live VM when running on VMX.  Userspace
	 * can modify the underlying VMA, and while doing so is beyond crazy,
	 * the cost of supporting that behavior is to simply retry the reload,
	 * which KVM should do anyways to avoid carrying a stale pfn pointer.
	 */