lkml.org 
[lkml]   [2020]   [Apr]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 1/2] KVM: Check validity of resolved slot when searching memslots
    On Tue,  7 Apr 2020 23:40:58 -0700
    Sean Christopherson <sean.j.christopherson@intel.com> wrote:

    > Check that the resolved slot (somewhat confusingly named 'start') is a
    > valid/allocated slot before doing the final comparison to see if the
    > specified gfn resides in the associated slot. The resolved slot can be
    > invalid if the binary search loop terminated because the search index
    > was incremented beyond the number of used slots.
    >
    > This bug has existed since the binary search algorithm was introduced,
    > but went unnoticed because KVM statically allocated memory for the max
    > number of slots, i.e. the access would only be truly out-of-bounds if
    > all possible slots were allocated and the specified gfn was less than
    > the base of the lowest memslot. Commit 36947254e5f98 ("KVM: Dynamically
    > size memslot array based on number of used slots") eliminated the "all
    > possible slots allocated" condition and made the bug embarrasingly easy
    > to hit.
    >
    > Fixes: 9c1a5d38780e6 ("kvm: optimize GFN to memslot lookup with large slots amount")
    > Reported-by: syzbot+d889b59b2bb87d4047a2@syzkaller.appspotmail.com
    > Cc: stable@vger.kernel.org
    > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
    > ---
    > include/linux/kvm_host.h | 2 +-
    > 1 file changed, 1 insertion(+), 1 deletion(-)
    >
    > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
    > index 6d58beb65454..01276e3d01b9 100644
    > --- a/include/linux/kvm_host.h
    > +++ b/include/linux/kvm_host.h
    > @@ -1048,7 +1048,7 @@ search_memslots(struct kvm_memslots *slots, gfn_t gfn)
    > start = slot + 1;
    > }
    >
    > - if (gfn >= memslots[start].base_gfn &&
    > + if (start < slots->used_slots && gfn >= memslots[start].base_gfn &&
    > gfn < memslots[start].base_gfn + memslots[start].npages) {
    > atomic_set(&slots->lru_slot, start);
    > return &memslots[start];

    Reviewed-by: Cornelia Huck <cohuck@redhat.com>

    \
     
     \ /
      Last update: 2020-04-08 09:04    [W:7.546 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site