lkml.org 
[lkml]   [2017]   [Sep]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Date
    Subject[PATCH 3.16 170/233] KVM: cpuid: Fix read/write out-of-bounds vulnerability in cpuid emulation
    3.16.48-rc1 review patch.  If anyone has any objections, please let me know.

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

    From: Wanpeng Li <wanpeng.li@hotmail.com>

    commit a3641631d14571242eec0d30c9faa786cbf52d44 upstream.

    If "i" is the last element in the vcpu->arch.cpuid_entries[] array, it
    potentially can be exploited the vulnerability. this will out-of-bounds
    read and write. Luckily, the effect is small:

    /* when no next entry is found, the current entry[i] is reselected */
    for (j = i + 1; ; j = (j + 1) % nent) {
    struct kvm_cpuid_entry2 *ej = &vcpu->arch.cpuid_entries[j];
    if (ej->function == e->function) {

    It reads ej->maxphyaddr, which is user controlled. However...

    ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;

    After cpuid_entries there is

    int maxphyaddr;
    struct x86_emulate_ctxt emulate_ctxt; /* 16-byte aligned */

    So we have:

    - cpuid_entries at offset 1B50 (6992)
    - maxphyaddr at offset 27D0 (6992 + 3200 = 10192)
    - padding at 27D4...27DF
    - emulate_ctxt at 27E0

    And it writes in the padding. Pfew, writing the ops field of emulate_ctxt
    would have been much worse.

    This patch fixes it by modding the index to avoid the out-of-bounds
    access. Worst case, i == j and ej->function == e->function,
    the loop can bail out.

    Reported-by: Moguofang <moguofang@huawei.com>
    Cc: Paolo Bonzini <pbonzini@redhat.com>
    Cc: Radim Krčmář <rkrcmar@redhat.com>
    Cc: Guofang Mo <moguofang@huawei.com>
    Signed-off-by: Wanpeng Li <wanpeng.li@hotmail.com>
    Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    arch/x86/kvm/cpuid.c | 20 +++++++++++---------
    1 file changed, 11 insertions(+), 9 deletions(-)

    --- a/arch/x86/kvm/cpuid.c
    +++ b/arch/x86/kvm/cpuid.c
    @@ -671,18 +671,20 @@ out:
    static int move_to_next_stateful_cpuid_entry(struct kvm_vcpu *vcpu, int i)
    {
    struct kvm_cpuid_entry2 *e = &vcpu->arch.cpuid_entries[i];
    - int j, nent = vcpu->arch.cpuid_nent;
    + struct kvm_cpuid_entry2 *ej;
    + int j = i;
    + int nent = vcpu->arch.cpuid_nent;

    e->flags &= ~KVM_CPUID_FLAG_STATE_READ_NEXT;
    /* when no next entry is found, the current entry[i] is reselected */
    - for (j = i + 1; ; j = (j + 1) % nent) {
    - struct kvm_cpuid_entry2 *ej = &vcpu->arch.cpuid_entries[j];
    - if (ej->function == e->function) {
    - ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
    - return j;
    - }
    - }
    - return 0; /* silence gcc, even though control never reaches here */
    + do {
    + j = (j + 1) % nent;
    + ej = &vcpu->arch.cpuid_entries[j];
    + } while (ej->function != e->function);
    +
    + ej->flags |= KVM_CPUID_FLAG_STATE_READ_NEXT;
    +
    + return j;
    }

    /* find an entry with matching function, matching index (if needed), and that
    \
     
     \ /
      Last update: 2017-09-10 01:29    [W:4.182 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site