mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Paolo Bonzini <pbonzini@redhat.com>
To: linux-kernel@vger.kernel.org, kvm@vger.kernel.org
Cc: Quentin Casasnovas <quentin.casasnovas@oracle.com>,
	stable@vger.kernel.org, Vegard Nossum <vegard.nossum@oracle.com>,
	Jamie Iles <jamie.iles@oracle.com>
Subject: [PATCH 13/14] kvm: fix excessive pages un-pinning in kvm_iommu_map error path.
Date: Fri, 24 Oct 2014 17:07:24 +0200	[thread overview]
Message-ID: <1414163245-18555-14-git-send-email-pbonzini@redhat.com> (raw)
In-Reply-To: <1414163245-18555-1-git-send-email-pbonzini@redhat.com>

From: Quentin Casasnovas <quentin.casasnovas@oracle.com>

The third parameter of kvm_unpin_pages() when called from
kvm_iommu_map_pages() is wrong, it should be the number of pages to un-pin
and not the page size.

This error was facilitated with an inconsistent API: kvm_pin_pages() takes
a size, but kvn_unpin_pages() takes a number of pages, so fix the problem
by matching the two.

This was introduced by commit 350b8bd ("kvm: iommu: fix the third parameter
of kvm_iommu_put_pages (CVE-2014-3601)"), which fixes the lack of
un-pinning for pages intended to be un-pinned (i.e. memory leak) but
unfortunately potentially aggravated the number of pages we un-pin that
should have stayed pinned. As far as I understand though, the same
practical mitigations apply.

This issue was found during review of Red Hat 6.6 patches to prepare
Ksplice rebootless updates.

Thanks to Vegard for his time on a late Friday evening to help me in
understanding this code.

Fixes: 350b8bd ("kvm: iommu: fix the third parameter of... (CVE-2014-3601)")
Cc: stable@vger.kernel.org
Signed-off-by: Quentin Casasnovas <quentin.casasnovas@oracle.com>
Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
Signed-off-by: Jamie Iles <jamie.iles@oracle.com>
Reviewed-by: Sasha Levin <sasha.levin@oracle.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/iommu.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index e51d9f9b995f..c1e6ae989a43 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -43,13 +43,13 @@ static void kvm_iommu_put_pages(struct kvm *kvm,
 				gfn_t base_gfn, unsigned long npages);
 
 static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
-			   unsigned long size)
+			   unsigned long npages)
 {
 	gfn_t end_gfn;
 	pfn_t pfn;
 
 	pfn     = gfn_to_pfn_memslot(slot, gfn);
-	end_gfn = gfn + (size >> PAGE_SHIFT);
+	end_gfn = gfn + npages;
 	gfn    += 1;
 
 	if (is_error_noslot_pfn(pfn))
@@ -119,7 +119,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		 * Pin all pages we are about to map in memory. This is
 		 * important because we unmap and unpin in 4kb steps later.
 		 */
-		pfn = kvm_pin_pages(slot, gfn, page_size);
+		pfn = kvm_pin_pages(slot, gfn, page_size >> PAGE_SHIFT);
 		if (is_error_noslot_pfn(pfn)) {
 			gfn += 1;
 			continue;
@@ -131,7 +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);
+			kvm_unpin_pages(kvm, pfn, page_size >> PAGE_SHIFT);
 			goto unmap_pages;
 		}
 
-- 
1.8.3.1



  parent reply	other threads:[~2014-10-24 15:08 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-24 15:07 [PATCH 00/14] KVM changes for 3.18-rc2 Paolo Bonzini
2014-10-24 15:07 ` [PATCH 01/14] KVM: x86: Check non-canonical addresses upon WRMSR Paolo Bonzini
2014-10-24 15:07 ` [PATCH 02/14] KVM: x86: Prevent host from panicking on shared MSR writes Paolo Bonzini
2014-10-24 15:07 ` [PATCH 03/14] KVM: x86: Improve thread safety in pit Paolo Bonzini
2014-10-24 15:07 ` [PATCH 04/14] KVM: x86: Fix wrong masking on relative jump/call Paolo Bonzini
2014-10-24 15:07 ` [PATCH 05/14] KVM: x86: Emulator fixes for eip canonical checks on near branches Paolo Bonzini
2014-10-24 17:53   ` Andy Lutomirski
2014-10-25 19:57     ` Nadav Amit
2014-10-25 23:51       ` Andy Lutomirski
2014-10-24 15:07 ` [PATCH 06/14] KVM: x86: Handle errors when RIP is set during far jumps Paolo Bonzini
2014-10-24 15:07 ` [PATCH 07/14] kvm: vmx: handle invvpid vm exit gracefully Paolo Bonzini
2014-10-24 15:07 ` [PATCH 08/14] kvm: x86: don't kill guest on unknown exit reason Paolo Bonzini
2014-10-24 17:57   ` Andy Lutomirski
2014-10-24 21:54     ` Paolo Bonzini
2014-10-24 22:26       ` Andy Lutomirski
2014-10-24 15:07 ` [PATCH 09/14] KVM: x86: Decoding guest instructions which cross page boundary may fail Paolo Bonzini
2014-10-24 15:07 ` [PATCH 10/14] KVM: emulate: avoid accessing NULL ctxt->memopp Paolo Bonzini
2014-10-24 15:07 ` [PATCH 11/14] KVM: x86: Emulator does not decode clflush well Paolo Bonzini
2014-10-24 15:07 ` [PATCH 12/14] KVM: x86: PREFETCH and HINT_NOP should have SrcMem flag Paolo Bonzini
2014-10-24 15:07 ` Paolo Bonzini [this message]
2014-10-24 15:58   ` [PATCH 13/14] kvm: fix excessive pages un-pinning in kvm_iommu_map error path Quentin Casasnovas
2014-10-24 15:07 ` [PATCH 14/14] KVM: x86: Wrong assertion on paging_tmpl.h Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1414163245-18555-14-git-send-email-pbonzini@redhat.com \
    --to=pbonzini@redhat.com \
    --cc=jamie.iles@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=quentin.casasnovas@oracle.com \
    --cc=stable@vger.kernel.org \
    --cc=vegard.nossum@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®