CVE-2013-1943: Improper Input Validation (CWE-20)
value: The KVM subsystem in the Linux kernel before 3.0 does not check whether kernel addresses are specified during allocation of memory slots for use in a guest's physical address space, which allows local users to gain privileges or obtain sensitive information from kernel memory via a crafted application, related to arch/x86/kvm/paging_tmpl.h and virt/kvm/kvm_main.c.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 7.8
Published: 2013-07-16T14:08Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: arch/x86/kvm/paging_tmpl.h, virt/kvm/kvm_main.c
Commit date: 2011-05-22 08:47:56 -0400
Commit message: KVM: Validate userspace_addr of memslot when registered

This way, we can avoid checking the user space address many times when
we read the guest memory.

Although we can do the same for write if we check which slots are
writable, we do not care write now: reading the guest memory happens
more often than writing.

[avi: change VERIFY_READ to VERIFY_WRITE]

Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Signed-off-by: Avi Kivity <avi@redhat.com>

Diff: diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h
index 88ca456ccd6801..e3f81418797e63 100644
--- a/arch/x86/kvm/paging_tmpl.h
+++ b/arch/x86/kvm/paging_tmpl.h
@@ -183,11 +183,11 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 			present = false;
 			break;
 		}
 
 		ptep_user = (pt_element_t __user *)((void *)host_addr + offset);
-		if (unlikely(copy_from_user(&pte, ptep_user, sizeof(pte)))) {
+		if (unlikely(__copy_from_user(&pte, ptep_user, sizeof(pte)))) {
 			present = false;
 			break;
 		}
 
 		trace_kvm_mmu_paging_element(pte, walker->level);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 58146457bf97fa..ed3c4e7c10089a 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -646,11 +646,14 @@ int __kvm_set_memory_region(struct kvm *kvm,
 	/* General sanity checks */
 	if (mem->memory_size & (PAGE_SIZE - 1))
 		goto out;
 	if (mem->guest_phys_addr & (PAGE_SIZE - 1))
 		goto out;
-	if (user_alloc && (mem->userspace_addr & (PAGE_SIZE - 1)))
+	/* We can read the guest memory with __xxx_user() later on. */
+	if (user_alloc &&
+	    ((mem->userspace_addr & (PAGE_SIZE - 1)) ||
+	     !access_ok(VERIFY_WRITE, mem->userspace_addr, mem->memory_size)))
 		goto out;
 	if (mem->slot >= KVM_MEMORY_SLOTS + KVM_PRIVATE_MEM_SLOTS)
 		goto out;
 	if (mem->guest_phys_addr + mem->memory_size < mem->guest_phys_addr)
 		goto out;
@@ -1281,11 +1284,11 @@ int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 	unsigned long addr;
 
 	addr = gfn_to_hva(kvm, gfn);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
-	r = copy_from_user(data, (void __user *)addr + offset, len);
+	r = __copy_from_user(data, (void __user *)addr + offset, len);
 	if (r)
 		return -EFAULT;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(kvm_read_guest_page);

Vulnerable code: 		if (unlikely(copy_from_user(&pte, ptep_user, sizeof(pte)))) {
	if (user_alloc && (mem->userspace_addr & (PAGE_SIZE - 1)))
	r = copy_from_user(data, (void __user *)addr + offset, len);
Fixed code: 		if (unlikely(__copy_from_user(&pte, ptep_user, sizeof(pte)))) {
	/* We can read the guest memory with __xxx_user() later on. */
	if (user_alloc &&
	    ((mem->userspace_addr & (PAGE_SIZE - 1)) ||
	     !access_ok(VERIFY_WRITE, mem->userspace_addr, mem->memory_size)))
	r = __copy_from_user(data, (void __user *)addr + offset, len);