linux-kernel - Re: [PATCH] KVM: prevent overlap between user and private memslots

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]

Date:   Fri, 19 Jan 2018 17:03:47 +0800
From:   Wanpeng Li <kernellwp@...il.com>
To:     Eric Biggers <ebiggers3@...il.com>
Cc:     kvm <kvm@...r.kernel.org>, Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>,
        linux-kernel@...r.kernel.org, syzkaller-bugs@...glegroups.com,
        Eric Biggers <ebiggers@...gle.com>,
        "# v3 . 10+" <stable@...r.kernel.org>
Subject: Re: [PATCH] KVM: prevent overlap between user and private memslots

2018-01-19 17:01 GMT+08:00 Wanpeng Li <kernellwp@...il.com>:
> 2018-01-19 16:18 GMT+08:00 Eric Biggers <ebiggers3@...il.com>:
>> From: Eric Biggers <ebiggers@...gle.com>
>>
>> Memslots must not overlap in guest physical memory, since otherwise some
>> guest physical addresses will not uniquely map to a memslot.  Yet, the
>> overlap check in __kvm_set_memory_region() allows a memslot that
>> overlaps one of the "private" memslots, e.g. the memslot reserved for
>> the TSS on x86.
>>
>> This seems to be a very old bug that was introduced years ago when
>> private memory slots were first added.  It seems that later refactoring
>> incorrectly assumed this bug was intentional and preserved it.
>>
>> Fix it by removing the loophole for private memslots, so we just check
>> for overlap against all memslots.
>>
>> This bug was found by syzkaller, which used a memslot overlap to make
>> pte_list_remove() be called for the wrong memslot, hitting a BUG():
>>
>>     pte_list_remove: 000000007185ed42 0->BUG
>>     kernel BUG at arch/x86/kvm/mmu.c:1209!
>>     [...]
>>     RIP: 0010:pte_list_remove+0x107/0x110 arch/x86/kvm/mmu.c:1208
>>     [...]
>>     Call Trace:
>>      mmu_page_zap_pte+0x7e/0xd0 arch/x86/kvm/mmu.c:2499
>>      kvm_mmu_page_unlink_children arch/x86/kvm/mmu.c:2521 [inline]
>>      kvm_mmu_prepare_zap_page+0x4f/0x340 arch/x86/kvm/mmu.c:2565
>>      kvm_zap_obsolete_pages arch/x86/kvm/mmu.c:5348 [inline]
>>      kvm_mmu_invalidate_zap_all_pages+0xa6/0x100 arch/x86/kvm/mmu.c:5389
>>      kvm_mmu_notifier_release+0x4f/0x80 arch/x86/kvm/../../../virt/kvm/kvm_main.c:468
>>      __mmu_notifier_release+0x63/0x100 mm/mmu_notifier.c:75
>>      mmu_notifier_release include/linux/mmu_notifier.h:244 [inline]
>>      exit_mmap+0x160/0x170 mm/mmap.c:3009
>>      __mmput kernel/fork.c:966 [inline]
>>      mmput+0x44/0xd0 kernel/fork.c:987
>>      exit_mm kernel/exit.c:544 [inline]
>>      do_exit+0x24a/0xb50 kernel/exit.c:856
>>      do_group_exit+0x34/0xb0 kernel/exit.c:972
>>      SYSC_exit_group kernel/exit.c:983 [inline]
>>      SyS_exit_group+0xb/0x10 kernel/exit.c:981
>>      entry_SYSCALL_64_fastpath+0x1e/0x8b
>>
>> Reproducer:
>>
>>     #include <fcntl.h>
>>     #include <linux/kvm.h>
>>     #include <sys/ioctl.h>
>>
>>     int main()
>>     {
>>             static char buf[4096*3] __attribute__((aligned(4096)));
>>             int kvm, vm, cpu;
>>             struct kvm_mp_state mp_state = { KVM_MP_STATE_SIPI_RECEIVED };
>>             struct kvm_userspace_memory_region memreg = {
>>                     .memory_size = sizeof(buf),
>>                     .userspace_addr = (__u64)buf,
>>             };
>>
>>             kvm = open("/dev/kvm", O_RDWR);
>>             vm = ioctl(kvm, KVM_CREATE_VM, 0);
>>             ioctl(vm, KVM_CREATE_IRQCHIP);
>>             cpu = ioctl(vm, KVM_CREATE_VCPU, 0);
>>             ioctl(cpu, KVM_SET_MP_STATE, &mp_state);
>>             ioctl(vm, KVM_SET_TSS_ADDR, 0);
>>             ioctl(cpu, KVM_RUN, 0);
>>             ioctl(vm, KVM_SET_USER_MEMORY_REGION, &memreg);
>>     }
>>
>> Reported-by: syzbot <syzkaller@...glegroups.com>
>> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
>> Cc: <stable@...r.kernel.org> # v2.6.25+
>> Signed-off-by: Eric Biggers <ebiggers@...gle.com>
>
> Please refer to this one. https://patchwork.kernel.org/patch/9645377/

https://lkml.org/lkml/2017/3/27/57

>
> Regards,
> Wanpeng Li
>
>> ---
>>  virt/kvm/kvm_main.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 210bf820385a..e536977e7b6d 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -974,8 +974,7 @@ int __kvm_set_memory_region(struct kvm *kvm,
>>                 /* Check for overlaps */
>>                 r = -EEXIST;
>>                 kvm_for_each_memslot(slot, __kvm_memslots(kvm, as_id)) {
>> -                       if ((slot->id >= KVM_USER_MEM_SLOTS) ||
>> -                           (slot->id == id))
>> +                       if (slot->id == id)
>>                                 continue;
>>                         if (!((base_gfn + npages <= slot->base_gfn) ||
>>                               (base_gfn >= slot->base_gfn + slot->npages)))
>> --
>> 2.16.0
>>