summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJim Mattson <jmattson@google.com>2018-12-17 22:53:33 +0100
committerPaolo Bonzini <pbonzini@redhat.com>2018-12-21 11:28:22 +0100
commitf1b9dd5eb86cec1fcf66aad17e7701d98d024a9a (patch)
treea3905b1d27e8e4514953b4f0aa1eec705fa30afd
parentKVM: VMX: Remove duplicated include from vmx.c (diff)
downloadlinux-f1b9dd5eb86cec1fcf66aad17e7701d98d024a9a.tar.xz
linux-f1b9dd5eb86cec1fcf66aad17e7701d98d024a9a.zip
kvm: Disallow wraparound in kvm_gfn_to_hva_cache_init
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@google.com> Signed-off-by: Jim Mattson <jmattson@google.com> Reviewed-by: Cfir Cohen <cfir@google.com> Reviewed-by: Marc Orr <marcorr@google.com> Cc: Andrew Honig <ahonig@google.com> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
-rw-r--r--virt/kvm/kvm_main.c41
1 files changed, 21 insertions, 20 deletions
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 0041947b7390..3be46841db06 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2005,32 +2005,33 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
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(slots, 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(slots, 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) {
- nr_pages_avail = 0;
- ghc->memslot = __gfn_to_memslot(slots, 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;
}
int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,