linux-kernel - [PATCH 3.16 75/99] kvm: Disallow wraparound in kvm_gfn_to_hva_cache

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1554212308.932552984@decadent.org.uk>
Date:   Tue, 02 Apr 2019 14:38:28 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org, Denis Kirjanov <kda@...ux-powerpc.org>,
        "Andrew Honig" <ahonig@...gle.com>,
        "Marc Orr" <marcorr@...gle.com>, "Cfir Cohen" <cfir@...gle.com>,
        "Radim Krčmář" <rkrcmar@...hat.com>,
        "Jim Mattson" <jmattson@...gle.com>
Subject: [PATCH 3.16 75/99] kvm: Disallow wraparound in kvm_gfn_to_hva_cache_init

3.16.65-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: Jim Mattson <jmattson@...gle.com>

commit f1b9dd5eb86cec1fcf66aad17e7701d98d024a9a upstream.

Previously, in the case where (gpa + len) wrapped around, the entire
region was not validated, as the comment claimed. It doesn't actually
seem that wraparound should be allowed here at all.

Furthermore, since some callers don't check the return code from this
function, it seems prudent to clear ghc->memslot in the event of an
error.

Fixes: 8f964525a121f ("KVM: Allow cross page reads and writes from cached translations.")
Reported-by: Cfir Cohen <cfir@...gle.com>
Signed-off-by: Jim Mattson <jmattson@...gle.com>
Reviewed-by: Cfir Cohen <cfir@...gle.com>
Reviewed-by: Marc Orr <marcorr@...gle.com>
Cc: Andrew Honig <ahonig@...gle.com>
Signed-off-by: Radim Krčmář <rkrcmar@...hat.com>
[bwh: Backported to 3.16: adjust context]
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1555,31 +1555,33 @@ int kvm_gfn_to_hva_cache_init(struct kvm
 	gfn_t end_gfn = (gpa + len - 1) >> PAGE_SHIFT;
 	gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
 	gfn_t nr_pages_avail;
+	int r = start_gfn <= end_gfn ? 0 : -EINVAL;
 
 	ghc->gpa = gpa;
 	ghc->generation = slots->generation;
 	ghc->len = len;
-	ghc->memslot = gfn_to_memslot(kvm, start_gfn);
-	ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn, NULL);
-	if (!kvm_is_error_hva(ghc->hva) && nr_pages_needed <= 1) {
+	ghc->hva = KVM_HVA_ERR_BAD;
+
+	/*
+	 * If the requested region crosses two memslots, we still
+	 * verify that the entire region is valid here.
+	 */
+	while (!r && start_gfn <= end_gfn) {
+		ghc->memslot = gfn_to_memslot(kvm, start_gfn);
+		ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn,
+					   &nr_pages_avail);
+		if (kvm_is_error_hva(ghc->hva))
+			r = -EFAULT;
+		start_gfn += nr_pages_avail;
+	}
+
+	/* Use the slow path for cross page reads and writes. */
+	if (!r && nr_pages_needed == 1)
 		ghc->hva += offset;
-	} else {
-		/*
-		 * If the requested region crosses two memslots, we still
-		 * verify that the entire region is valid here.
-		 */
-		while (start_gfn <= end_gfn) {
-			ghc->memslot = gfn_to_memslot(kvm, start_gfn);
-			ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn,
-						   &nr_pages_avail);
-			if (kvm_is_error_hva(ghc->hva))
-				return -EFAULT;
-			start_gfn += nr_pages_avail;
-		}
-		/* Use the slow path for cross page reads and writes. */
+	else
 		ghc->memslot = NULL;
-	}
-	return 0;
+
+	return r;
 }
 EXPORT_SYMBOL_GPL(kvm_gfn_to_hva_cache_init);