Skip to content

Commit 1f74d79

Browse files
Wanpeng Ligregkh
authored andcommitted
KVM: X86: Fix userspace set invalid CR4
commit 3ca9419 upstream. Reported by syzkaller: WARNING: CPU: 0 PID: 6544 at /home/kernel/data/kvm/arch/x86/kvm//vmx/vmx.c:4689 handle_desc+0x37/0x40 [kvm_intel] CPU: 0 PID: 6544 Comm: a.out Tainted: G OE 5.3.0-rc4+ #4 RIP: 0010:handle_desc+0x37/0x40 [kvm_intel] Call Trace: vmx_handle_exit+0xbe/0x6b0 [kvm_intel] vcpu_enter_guest+0x4dc/0x18d0 [kvm] kvm_arch_vcpu_ioctl_run+0x407/0x660 [kvm] kvm_vcpu_ioctl+0x3ad/0x690 [kvm] do_vfs_ioctl+0xa2/0x690 ksys_ioctl+0x6d/0x80 __x64_sys_ioctl+0x1a/0x20 do_syscall_64+0x74/0x720 entry_SYSCALL_64_after_hwframe+0x49/0xbe When CR4.UMIP is set, guest should have UMIP cpuid flag. Current kvm set_sregs function doesn't have such check when userspace inputs sregs values. SECONDARY_EXEC_DESC is enabled on writes to CR4.UMIP in vmx_set_cr4 though guest doesn't have UMIP cpuid flag. The testcast triggers handle_desc warning when executing ltr instruction since guest architectural CR4 doesn't set UMIP. This patch fixes it by adding valid CR4 and CPUID combination checking in __set_sregs. syzkaller source: https://syzkaller.appspot.com/x/repro.c?x=138efb99600000 Reported-by: syzbot+0f1819555fbdce992df9@syzkaller.appspotmail.com Cc: stable@vger.kernel.org Signed-off-by: Wanpeng Li <wanpengli@tencent.com> Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent b1344c6 commit 1f74d79

File tree

1 file changed

+21
-17
lines changed

1 file changed

+21
-17
lines changed

arch/x86/kvm/x86.c

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -884,34 +884,42 @@ int kvm_set_xcr(struct kvm_vcpu *vcpu, u32 index, u64 xcr)
884884
}
885885
EXPORT_SYMBOL_GPL(kvm_set_xcr);
886886

887-
int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
887+
static int kvm_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
888888
{
889-
unsigned long old_cr4 = kvm_read_cr4(vcpu);
890-
unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
891-
X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
892-
893889
if (cr4 & CR4_RESERVED_BITS)
894-
return 1;
890+
return -EINVAL;
895891

896892
if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) && (cr4 & X86_CR4_OSXSAVE))
897-
return 1;
893+
return -EINVAL;
898894

899895
if (!guest_cpuid_has(vcpu, X86_FEATURE_SMEP) && (cr4 & X86_CR4_SMEP))
900-
return 1;
896+
return -EINVAL;
901897

902898
if (!guest_cpuid_has(vcpu, X86_FEATURE_SMAP) && (cr4 & X86_CR4_SMAP))
903-
return 1;
899+
return -EINVAL;
904900

905901
if (!guest_cpuid_has(vcpu, X86_FEATURE_FSGSBASE) && (cr4 & X86_CR4_FSGSBASE))
906-
return 1;
902+
return -EINVAL;
907903

908904
if (!guest_cpuid_has(vcpu, X86_FEATURE_PKU) && (cr4 & X86_CR4_PKE))
909-
return 1;
905+
return -EINVAL;
910906

911907
if (!guest_cpuid_has(vcpu, X86_FEATURE_LA57) && (cr4 & X86_CR4_LA57))
912-
return 1;
908+
return -EINVAL;
913909

914910
if (!guest_cpuid_has(vcpu, X86_FEATURE_UMIP) && (cr4 & X86_CR4_UMIP))
911+
return -EINVAL;
912+
913+
return 0;
914+
}
915+
916+
int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
917+
{
918+
unsigned long old_cr4 = kvm_read_cr4(vcpu);
919+
unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
920+
X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
921+
922+
if (kvm_valid_cr4(vcpu, cr4))
915923
return 1;
916924

917925
if (is_long_mode(vcpu)) {
@@ -8598,10 +8606,6 @@ EXPORT_SYMBOL_GPL(kvm_task_switch);
85988606

85998607
static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
86008608
{
8601-
if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
8602-
(sregs->cr4 & X86_CR4_OSXSAVE))
8603-
return -EINVAL;
8604-
86058609
if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG)) {
86068610
/*
86078611
* When EFER.LME and CR0.PG are set, the processor is in
@@ -8620,7 +8624,7 @@ static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
86208624
return -EINVAL;
86218625
}
86228626

8623-
return 0;
8627+
return kvm_valid_cr4(vcpu, sregs->cr4);
86248628
}
86258629

86268630
static int __set_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)

0 commit comments

Comments
 (0)