CVE-2014-3601: Numeric Errors (CWE-189)
value: The kvm_iommu_map_pages function in virt/kvm/iommu.c in the Linux kernel through 3.16.1 miscalculates the number of pages during the handling of a mapping failure, which allows guest OS users to (1) cause a denial of service (host OS memory corruption) or possibly have unspecified other impact by triggering a large gfn value or (2) cause a denial of service (host OS memory consumption) by triggering a small gfn value that leads to permanently pinned pages.
CWE description: Weaknesses in this category are related to improper calculation or conversion of numbers.
Published: 2014-09-01T01:55Z
Repository: https://github.com/torvalds/linux
Language: C
Files changed: virt/kvm/iommu.c
Commit date: 2014-08-19 15:04:45 +0200
Commit message: kvm: iommu: fix the third parameter of kvm_iommu_put_pages (CVE-2014-3601)

The third parameter of kvm_iommu_put_pages is wrong,
It should be 'gfn - slot->base_gfn'.

By making gfn very large, malicious guest or userspace can cause kvm to
go to this error path, and subsequently to pass a huge value as size.
Alternatively if gfn is small, then pages would be pinned but never
unpinned, causing host memory leak and local DOS.

Passing a reasonable but large value could be the most dangerous case,
because it would unpin a page that should have stayed pinned, and thus
allow the device to DMA into arbitrary memory.  However, this cannot
happen because of the condition that can trigger the error:

- out of memory (where you can't allocate even a single page)
  should not be possible for the attacker to trigger

- when exceeding the iommu's address space, guest pages after gfn
  will also exceed the iommu's address space, and inside
  kvm_iommu_put_pages() the iommu_iova_to_phys() will fail.  The
  page thus would not be unpinned at all.

Reported-by: Jack Morgenstein <jackm@mellanox.com>
Cc: stable@vger.kernel.org
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>

Diff: diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 0df7d4b34dfec9..714b949323120a 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -59,10 +59,18 @@ static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
 		gfn_to_pfn_memslot(slot, gfn++);
 
 	return pfn;
 }
 
+static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
+{
+	unsigned long i;
+
+	for (i = 0; i < npages; ++i)
+		kvm_release_pfn_clean(pfn + i);
+}
+
 int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 {
 	gfn_t gfn, end_gfn;
 	pfn_t pfn;
 	int r = 0;
@@ -121,10 +129,11 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		r = iommu_map(domain, gfn_to_gpa(gfn), pfn_to_hpa(pfn),
 			      page_size, flags);
 		if (r) {
 			printk(KERN_ERR "kvm_iommu_map_address:"
 			       "iommu failed to map pfn=%llx\n", pfn);
+			kvm_unpin_pages(kvm, pfn, page_size);
 			goto unmap_pages;
 		}
 
 		gfn += page_size >> PAGE_SHIFT;
 
@@ -132,11 +141,11 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 	}
 
 	return 0;
 
 unmap_pages:
-	kvm_iommu_put_pages(kvm, slot->base_gfn, gfn);
+	kvm_iommu_put_pages(kvm, slot->base_gfn, gfn - slot->base_gfn);
 	return r;
 }
 
 static int kvm_iommu_map_memslots(struct kvm *kvm)
 {
@@ -264,18 +273,10 @@ int kvm_iommu_map_guest(struct kvm *kvm)
 out_unlock:
 	mutex_unlock(&kvm->slots_lock);
 	return r;
 }
 
-static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
-{
-	unsigned long i;
-
-	for (i = 0; i < npages; ++i)
-		kvm_release_pfn_clean(pfn + i);
-}
-
 static void kvm_iommu_put_pages(struct kvm *kvm,
 				gfn_t base_gfn, unsigned long npages)
 {
 	struct iommu_domain *domain;
 	gfn_t end_gfn, gfn;

Vulnerable code: 	kvm_iommu_put_pages(kvm, slot->base_gfn, gfn);
static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
{
	unsigned long i;

	for (i = 0; i < npages; ++i)
		kvm_release_pfn_clean(pfn + i);
}

Fixed code: static void kvm_unpin_pages(struct kvm *kvm, pfn_t pfn, unsigned long npages)
{
	unsigned long i;

	for (i = 0; i < npages; ++i)
		kvm_release_pfn_clean(pfn + i);
}

			kvm_unpin_pages(kvm, pfn, page_size);
	kvm_iommu_put_pages(kvm, slot->base_gfn, gfn - slot->base_gfn);