[0/4] KVM: x86: Add checks on host-reserved cr4 bits

Message

Sean Christopherson Dec. 10, 2019, 10:44 p.m. UTC

KVM currently doesn't incorporate the platform's capabilities in its cr4
reserved bit checks and instead checks only whether KVM is aware of the
feature in general and whether or not userspace has advertised the feature
to the guest.

Lack of checking allows userspace/guest to set unsupported bits in cr4.
For the most part, setting unsupported bits will simply cause VM-Enter to
fail.  The one existing exception is OSXSAVE, which is conditioned on host
support as checking only guest_cpuid_has() would result in KVM attempting
XSAVE, leading to faults and WARNs.

57-bit virtual addressing has introduced another case where setting an
unsupported bit (cr4.LA57) can induce a fault in the host.  In the LA57
case, userspace can set the guest's cr4.LA57 by advertising LA57 support
via CPUID and abuse the bogus cr4.LA57 to effectively bypass KVM's
non-canonical address check, ultimately causing a #GP when VMX writes
the guest's bogus address to MSR_KERNEL_GS_BASE during VM-Enter.

Given that the best case scenario is a failed VM-Enter, there's no sane
reason to allow setting unsupported bits in cr4.  Fix the LA57 bug by not
allowing userspace or the guest to set cr4 bits that are not supported
by the platform.

Sean Christopherson (4):
  KVM: x86: Don't let userspace set host-reserved cr4 bits
  KVM: x86: Ensure all logical CPUs have consistent reserved cr4 bits
  KVM: x86: Drop special XSAVE handling from guest_cpuid_has()
  KVM: x86: Add macro to ensure reserved cr4 bits checks stay in sync

 arch/x86/kvm/cpuid.h   |  4 ---
 arch/x86/kvm/svm.c     |  1 +
 arch/x86/kvm/vmx/vmx.c |  1 +
 arch/x86/kvm/x86.c     | 65 +++++++++++++++++++++++++++++-------------
 4 files changed, 47 insertions(+), 24 deletions(-)

Comments

Paolo Bonzini Jan. 15, 2020, 6:05 p.m. UTC | #1

On 10/12/19 23:44, Sean Christopherson wrote:
> KVM currently doesn't incorporate the platform's capabilities in its cr4
> reserved bit checks and instead checks only whether KVM is aware of the
> feature in general and whether or not userspace has advertised the feature
> to the guest.
> 
> Lack of checking allows userspace/guest to set unsupported bits in cr4.
> For the most part, setting unsupported bits will simply cause VM-Enter to
> fail.  The one existing exception is OSXSAVE, which is conditioned on host
> support as checking only guest_cpuid_has() would result in KVM attempting
> XSAVE, leading to faults and WARNs.
> 
> 57-bit virtual addressing has introduced another case where setting an
> unsupported bit (cr4.LA57) can induce a fault in the host.  In the LA57
> case, userspace can set the guest's cr4.LA57 by advertising LA57 support
> via CPUID and abuse the bogus cr4.LA57 to effectively bypass KVM's
> non-canonical address check, ultimately causing a #GP when VMX writes
> the guest's bogus address to MSR_KERNEL_GS_BASE during VM-Enter.
> 
> Given that the best case scenario is a failed VM-Enter, there's no sane
> reason to allow setting unsupported bits in cr4.  Fix the LA57 bug by not
> allowing userspace or the guest to set cr4 bits that are not supported
> by the platform.
> 
> Sean Christopherson (4):
>   KVM: x86: Don't let userspace set host-reserved cr4 bits
>   KVM: x86: Ensure all logical CPUs have consistent reserved cr4 bits
>   KVM: x86: Drop special XSAVE handling from guest_cpuid_has()
>   KVM: x86: Add macro to ensure reserved cr4 bits checks stay in sync
> 
>  arch/x86/kvm/cpuid.h   |  4 ---
>  arch/x86/kvm/svm.c     |  1 +
>  arch/x86/kvm/vmx/vmx.c |  1 +
>  arch/x86/kvm/x86.c     | 65 +++++++++++++++++++++++++++++-------------
>  4 files changed, 47 insertions(+), 24 deletions(-)
> 

Queued, thanks.

Paolo