3.16.65-rc1 review patch. If anyone has any objections, please let me know.
------------------
From: Jim Mattson jmattson@google.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@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 [bwh: Backported to 3.16: adjust context] Signed-off-by: Ben Hutchings ben@decadent.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);