Vulnerability: arch/arm/kvm/arm.c in the Linux kernel before 3.10 on the ARM platform, when KVM is used, allows host OS users to cause a denial of service (NULL pointer dereference, OOPS, and host OS crash) or possibly have unspecified other impact by omitting vCPU initialization before a KVM_GET_REG_LIST ioctl call.
Vulnerability ID: CVE-2013-5634
CWE: CWE-399
Severity score: 6.9
Project: https://github.com/torvalds/linux
Published: 2013-09-25
File: arch/arm/kvm/arm.c
Commit message: ARM: KVM: prevent NULL pointer dereferences with KVM VCPU ioctl

Some ARM KVM VCPU ioctls require the vCPU to be properly initialized
with the KVM_ARM_VCPU_INIT ioctl before being used with further
requests. KVM_RUN checks whether this initialization has been
done, but other ioctls do not.
Namely KVM_GET_REG_LIST will dereference an array with index -1
without initialization and thus leads to a kernel oops.
Fix this by adding checks before executing the ioctl handlers.

 [ Removed superflous comment from static function - Christoffer ]

Changes from v1:
 * moved check into a static function with a meaningful name

Signed-off-by: Andre Przywara <andre.przywara@linaro.org>
Signed-off-by: Christoffer Dall <cdall@cs.columbia.edu>
Commit author: Andre Przywara

Vulnerable version (before fix):
 static void vcpu_pause(struct kvm_vcpu *vcpu) 	wait_event_interruptible(*wq, !vcpu->arch.pause); }  /**  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code  * @vcpu:	The VCPU pointer int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) 	int ret; 	sigset_t sigsaved;  	/* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */ 	if (unlikely(vcpu->arch.target < 0)) 		return -ENOEXEC;  	ret = kvm_vcpu_first_run_init(vcpu); long kvm_arch_vcpu_ioctl(struct file *filp, 	case KVM_SET_ONE_REG: 	case KVM_GET_ONE_REG: { 		struct kvm_one_reg reg; 		if (copy_from_user(&reg, argp, sizeof(reg))) 			return -EFAULT; 		if (ioctl == KVM_SET_ONE_REG) long kvm_arch_vcpu_ioctl(struct file *filp, 		struct kvm_reg_list reg_list; 		unsigned n;  		if (copy_from_user(&reg_list, user_list, sizeof(reg_list))) 			return -EFAULT; 		n = reg_list.n;

Fixed version (after fix):
 static void vcpu_pause(struct kvm_vcpu *vcpu) 	wait_event_interruptible(*wq, !vcpu->arch.pause); }  static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu) { 	return vcpu->arch.target >= 0; }  /**  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code  * @vcpu:	The VCPU pointer int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run) 	int ret; 	sigset_t sigsaved;  	if (unlikely(!kvm_vcpu_initialized(vcpu))) 		return -ENOEXEC;  	ret = kvm_vcpu_first_run_init(vcpu); long kvm_arch_vcpu_ioctl(struct file *filp, 	case KVM_SET_ONE_REG: 	case KVM_GET_ONE_REG: { 		struct kvm_one_reg reg;  		if (unlikely(!kvm_vcpu_initialized(vcpu))) 			return -ENOEXEC;  		if (copy_from_user(&reg, argp, sizeof(reg))) 			return -EFAULT; 		if (ioctl == KVM_SET_ONE_REG) long kvm_arch_vcpu_ioctl(struct file *filp, 		struct kvm_reg_list reg_list; 		unsigned n;  		if (unlikely(!kvm_vcpu_initialized(vcpu))) 			return -ENOEXEC;  		if (copy_from_user(&reg_list, user_list, sizeof(reg_list))) 			return -EFAULT; 		n = reg_list.n;

Patch:
patch: @@ -492,6 +492,11 @@ static void vcpu_pause(struct kvm_vcpu *vcpu)
 	wait_event_interruptible(*wq, !vcpu->arch.pause);
 }
 
+static int kvm_vcpu_initialized(struct kvm_vcpu *vcpu)
+{
+	return vcpu->arch.target >= 0;
+}
+
 /**
  * kvm_arch_vcpu_ioctl_run - the main VCPU run function to execute guest code
  * @vcpu:	The VCPU pointer
@@ -508,8 +513,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
 	int ret;
 	sigset_t sigsaved;
 
-	/* Make sure they initialize the vcpu with KVM_ARM_VCPU_INIT */
-	if (unlikely(vcpu->arch.target < 0))
+	if (unlikely(!kvm_vcpu_initialized(vcpu)))
 		return -ENOEXEC;
 
 	ret = kvm_vcpu_first_run_init(vcpu);
@@ -710,6 +714,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	case KVM_SET_ONE_REG:
 	case KVM_GET_ONE_REG: {
 		struct kvm_one_reg reg;
+
+		if (unlikely(!kvm_vcpu_initialized(vcpu)))
+			return -ENOEXEC;
+
 		if (copy_from_user(&reg, argp, sizeof(reg)))
 			return -EFAULT;
 		if (ioctl == KVM_SET_ONE_REG)
@@ -722,6 +730,9 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		struct kvm_reg_list reg_list;
 		unsigned n;
 
+		if (unlikely(!kvm_vcpu_initialized(vcpu)))
+			return -ENOEXEC;
+
 		if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
 			return -EFAULT;
 		n = reg_list.n;
path: arch/arm/kvm/arm.c