Skip to content

Commit 2187402

Browse files
Wanpeng Ligregkh
Wanpeng Li
authored and
gregkh
committed
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: [email protected] Cc: [email protected] Signed-off-by: Wanpeng Li <[email protected]> Reviewed-by: Sean Christopherson <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]> Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 30fbe0d commit 2187402

1 file changed

Lines changed: 21 additions & 17 deletions

File tree

‎arch/x86/kvm/x86.c‎

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

794-
int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
794+
static int kvm_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
795795
{
796-
unsigned long old_cr4 = kvm_read_cr4(vcpu);
797-
unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
798-
X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
799-
800796
if (cr4 & CR4_RESERVED_BITS)
801-
return 1;
797+
return -EINVAL;
802798

803799
if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) && (cr4 & X86_CR4_OSXSAVE))
804-
return 1;
800+
return -EINVAL;
805801

806802
if (!guest_cpuid_has(vcpu, X86_FEATURE_SMEP) && (cr4 & X86_CR4_SMEP))
807-
return 1;
803+
return -EINVAL;
808804

809805
if (!guest_cpuid_has(vcpu, X86_FEATURE_SMAP) && (cr4 & X86_CR4_SMAP))
810-
return 1;
806+
return -EINVAL;
811807

812808
if (!guest_cpuid_has(vcpu, X86_FEATURE_FSGSBASE) && (cr4 & X86_CR4_FSGSBASE))
813-
return 1;
809+
return -EINVAL;
814810

815811
if (!guest_cpuid_has(vcpu, X86_FEATURE_PKU) && (cr4 & X86_CR4_PKE))
816-
return 1;
812+
return -EINVAL;
817813

818814
if (!guest_cpuid_has(vcpu, X86_FEATURE_LA57) && (cr4 & X86_CR4_LA57))
819-
return 1;
815+
return -EINVAL;
820816

821817
if (!guest_cpuid_has(vcpu, X86_FEATURE_UMIP) && (cr4 & X86_CR4_UMIP))
818+
return -EINVAL;
819+
820+
return 0;
821+
}
822+
823+
int kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4)
824+
{
825+
unsigned long old_cr4 = kvm_read_cr4(vcpu);
826+
unsigned long pdptr_bits = X86_CR4_PGE | X86_CR4_PSE | X86_CR4_PAE |
827+
X86_CR4_SMEP | X86_CR4_SMAP | X86_CR4_PKE;
828+
829+
if (kvm_valid_cr4(vcpu, cr4))
822830
return 1;
823831

824832
if (is_long_mode(vcpu)) {
@@ -8237,10 +8245,6 @@ EXPORT_SYMBOL_GPL(kvm_task_switch);
82378245

82388246
static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
82398247
{
8240-
if (!guest_cpuid_has(vcpu, X86_FEATURE_XSAVE) &&
8241-
(sregs->cr4 & X86_CR4_OSXSAVE))
8242-
return -EINVAL;
8243-
82448248
if ((sregs->efer & EFER_LME) && (sregs->cr0 & X86_CR0_PG)) {
82458249
/*
82468250
* When EFER.LME and CR0.PG are set, the processor is in
@@ -8259,7 +8263,7 @@ static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
82598263
return -EINVAL;
82608264
}
82618265

8262-
return 0;
8266+
return kvm_valid_cr4(vcpu, sregs->cr4);
82638267
}
82648268

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

0 commit comments

Comments
 (0)