[PATCH] KVM: x86/hyperv: Explicitly reject VP Assist Page reads if cache was clobbered

[PATCH] KVM: x86/hyperv: Explicitly reject VP Assist Page reads if cache was clobbered

[Sean Christopherson]

Don't try to read the VP Assist Page if the shared PV EOI cache has been
reconfigured via KVM's actual PV EOI MSR, as trying to read 624 bytes of
info when the cache is setup for only 1 byte will trip the sanity check
WARN in kvm_read_guest_offset_cached().  Ideally, KVM wouldn't use shared
state to track the VP Assist Page, but in practice no sane guest enables
both KVM's PV EOI and Hyper-V's Assist Page at different addresses, i.e.
not allowing both to co-exist independently is "fine".

Fixes: b63cf42fd1d8 ("kvm/x86: implement hv EOI assist")
Cc: stable@vger.kernel.org
Reported-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
Closes: https://lore.kernel.org/all/20261017093016.32348-7-amirmohammad.eftekhar@cispa.de
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/hyperv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 5d2e3617c952..c08eeaac8bb7 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -949,6 +949,15 @@ int kvm_hv_get_assist_page(struct kvm_vcpu *vcpu)
 	if (!hv_vcpu || !kvm_hv_assist_page_enabled(vcpu))
 		return -EFAULT;
 
+	/*
+	 * KVM reuses its own PV EOI feature to emulate Hyper-V's EOI Assist,
+	 * and rather stupidly reuses the same MSR and cache storage.  Treat
+	 * the assist page as inaccessible if the shared state is configured
+	 * only for PV EOI's single byte of data.
+	 */
+	if (vcpu->arch.pv_eoi.data.len < sizeof(struct hv_vp_assist_page))
+		return -EFAULT;
+
 	return kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.pv_eoi.data,
 				     &hv_vcpu->vp_assist_page, sizeof(struct hv_vp_assist_page));
 }

base-commit: 814607cd40df80d5727957adaa857e0feca43497
-- 
2.56.0.218.g1ec904b173-goog

Re: [PATCH] KVM: x86/hyperv: Explicitly reject VP Assist Page reads if cache was clobbered

[Vitaly Kuznetsov]

Sean Christopherson <seanjc@google.com> writes:

> Don't try to read the VP Assist Page if the shared PV EOI cache has been
> reconfigured via KVM's actual PV EOI MSR, as trying to read 624 bytes of
> info when the cache is setup for only 1 byte will trip the sanity check
> WARN in kvm_read_guest_offset_cached().  Ideally, KVM wouldn't use shared
> state to track the VP Assist Page, but in practice no sane guest enables
> both KVM's PV EOI and Hyper-V's Assist Page at different addresses, i.e.
> not allowing both to co-exist independently is "fine".
>
> Fixes: b63cf42fd1d8 ("kvm/x86: implement hv EOI assist")
> Cc: stable@vger.kernel.org
> Reported-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
> Closes: https://lore.kernel.org/all/20261017093016.32348-7-amirmohammad.eftekhar@cispa.de
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>

(I am, however, thinking it would be better in the long term to either
separate KVM PV / Hyper-V PV state here or even consider a 'big
hammer' and make these two (+Xen?) mutually exclusive for all PV
features.)

> ---
>  arch/x86/kvm/hyperv.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 5d2e3617c952..c08eeaac8bb7 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -949,6 +949,15 @@ int kvm_hv_get_assist_page(struct kvm_vcpu *vcpu)
>  	if (!hv_vcpu || !kvm_hv_assist_page_enabled(vcpu))
>  		return -EFAULT;
>  
> +	/*
> +	 * KVM reuses its own PV EOI feature to emulate Hyper-V's EOI Assist,
> +	 * and rather stupidly reuses the same MSR and cache storage.  Treat
> +	 * the assist page as inaccessible if the shared state is configured
> +	 * only for PV EOI's single byte of data.
> +	 */
> +	if (vcpu->arch.pv_eoi.data.len < sizeof(struct hv_vp_assist_page))
> +		return -EFAULT;
> +
>  	return kvm_read_guest_cached(vcpu->kvm, &vcpu->arch.pv_eoi.data,
>  				     &hv_vcpu->vp_assist_page, sizeof(struct hv_vp_assist_page));
>  }
>
> base-commit: 814607cd40df80d5727957adaa857e0feca43497

-- 
Vitaly

Re: [PATCH] KVM: x86/hyperv: Explicitly reject VP Assist Page reads if cache was clobbered

[Sean Christopherson]

On Thu, Oct 22, 2026, Vitaly Kuznetsov wrote:
> Sean Christopherson <seanjc@google.com> writes:
> 
> > Don't try to read the VP Assist Page if the shared PV EOI cache has been
> > reconfigured via KVM's actual PV EOI MSR, as trying to read 624 bytes of
> > info when the cache is setup for only 1 byte will trip the sanity check
> > WARN in kvm_read_guest_offset_cached().  Ideally, KVM wouldn't use shared
> > state to track the VP Assist Page, but in practice no sane guest enables
> > both KVM's PV EOI and Hyper-V's Assist Page at different addresses, i.e.
> > not allowing both to co-exist independently is "fine".
> >
> > Fixes: b63cf42fd1d8 ("kvm/x86: implement hv EOI assist")
> > Cc: stable@vger.kernel.org
> > Reported-by: Amirmohammad Eftekhar <amirmohammad.eftekhar@cispa.de>
> > Closes: https://lore.kernel.org/all/20261017093016.32348-7-amirmohammad.eftekhar@cispa.de
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> 
> Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> (I am, however, thinking it would be better in the long term to either
> separate KVM PV / Hyper-V PV state here or even consider a 'big
> hammer' and make these two (+Xen?) mutually exclusive for all PV
> features.)

I generally agree, the problem is that any changes could break guests.  For
whatever reason, kvm_lapic_set_pv_eoi() very deliberately allows the two features
to overlay.  I.e. while it seems absurd, in theory a guest could rely on being to
use the two features simultaneously.

OMG, it's your fault!  LOL.  (Obviously not actually your fault since you were
just trying to clean up the mess created by the initial implementation).

  Author:     Vitaly Kuznetsov <vkuznets@redhat.com>
  AuthorDate: Tue Oct 16 18:50:06 2018 +0200
  Commit:     Paolo Bonzini <pbonzini@redhat.com>
  CommitDate: Wed Oct 17 00:30:17 2018 +0200

    x86/kvm/lapic: preserve gfn_to_hva_cache len on cache reinit
    
    vcpu->arch.pv_eoi is accessible through both HV_X64_MSR_VP_ASSIST_PAGE and
    MSR_KVM_PV_EOI_EN so on migration userspace may try to restore them in any
    order. Values match, however, kvm_lapic_enable_pv_eoi() uses different
    length: for Hyper-V case it's the whole struct hv_vp_assist_page, for KVM
    native case it is 8. In case we restore KVM-native MSR last cache will
    be reinitialized with len=8 so trying to access VP assist page beyond
    8 bytes with kvm_read_guest_cached() will fail.
    
    Check if we re-initializing cache for the same address and preserve length
    in case it was greater.
    
    Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

So yeah, it definitely seems like we can separate the two since odds are very good
the only reason they "co-exist" is pbecause KVM tricked userspace into effectively
saving/restoring garbage via MSR_KVM_PV_EOI_EN.