diff options
author | Michael S. Tsirkin <mst@redhat.com> | 2014-08-19 19:14:50 +0800 |
---|---|---|
committer | Paolo Bonzini <pbonzini@redhat.com> | 2014-08-19 15:04:45 +0200 |
commit | 350b8bdd689cd2ab2c67c8a86a0be86cfa0751a7 (patch) | |
tree | 00f9822472c91756bf1d4c6577972e1d7d582823 | |
parent | 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9 (diff) | |
download | op-kernel-dev-350b8bdd689cd2ab2c67c8a86a0be86cfa0751a7.zip op-kernel-dev-350b8bdd689cd2ab2c67c8a86a0be86cfa0751a7.tar.gz |
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>
-rw-r--r-- | virt/kvm/iommu.c | 19 |
1 files changed, 10 insertions, 9 deletions
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c index 0df7d4b..714b949 100644 --- a/virt/kvm/iommu.c +++ b/virt/kvm/iommu.c @@ -61,6 +61,14 @@ static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t 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; @@ -123,6 +131,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot) 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; } @@ -134,7 +143,7 @@ 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; } @@ -266,14 +275,6 @@ out_unlock: 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) { |