Re: [PATCH] KVM: prevent overlap between user and private memslots
On 13/02/2018 04:38, Eric Biggers wrote:
> On Tue, Jan 30, 2018 at 12:49:21PM -0800, Eric Biggers wrote:
>> On Fri, Jan 19, 2018 at 10:57:16AM -0800, Eric Biggers wrote:
>>> +Cc alex.william...@redhat.com
>>>
>>> On Fri, Jan 19, 2018 at 05:03:47PM +0800, Wanpeng Li wrote:
2018-01-19 17:01 GMT+08:00 Wanpeng Li :
> 2018-01-19 16:18 GMT+08:00 Eric Biggers :
>> From: Eric Biggers
>>
>> 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: 7185ed42 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
>> #include
>> #include
>>
>> 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
>> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
>> Cc: # v2.6.25+
>> Signed-off-by: Eric Biggers
>
> Please refer to this one. https://patchwork.kernel.org/patch/9645377/
https://lkml.org/lkml/2017/3/27/57
>>>
>>> Ah, so this was reported before, and you sent the same fix. Well, it was
>>> never
>>> applied, so the bug is still there, and anyone who can use /dev/kvm can
>>> trigger
>>> it. So one of these patches needs to be applied, unless there is a better
>>> fix.
>>>
>>> I don't agree with the "Fixes:" line in your version of the patch. The bug
>>> was
>>> actually there prior to 5419369ed, which might explain why that commit
>>> seemed to
>>> preserve the behavior intentionally. (Note that KVM_MEMORY_SLOTS did not
>>> include the private memory slots; it was later renamed to
>>> KVM_USER_MEM_SLOTS.)
>>>
>>> Eric
>>
>> Ping. Paolo or Radim, can you please consider applying one of these patches?
Applied, thanks.
Paolo
Re: [PATCH] KVM: prevent overlap between user and private memslots
On Tue, Jan 30, 2018 at 12:49:21PM -0800, Eric Biggers wrote:
> On Fri, Jan 19, 2018 at 10:57:16AM -0800, Eric Biggers wrote:
> > +Cc alex.william...@redhat.com
> >
> > On Fri, Jan 19, 2018 at 05:03:47PM +0800, Wanpeng Li wrote:
> > > 2018-01-19 17:01 GMT+08:00 Wanpeng Li :
> > > > 2018-01-19 16:18 GMT+08:00 Eric Biggers :
> > > >> From: Eric Biggers
> > > >>
> > > >> 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: 7185ed42 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
> > > >> #include
> > > >> #include
> > > >>
> > > >> 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
> > > >> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
> > > >> Cc: # v2.6.25+
> > > >> Signed-off-by: Eric Biggers
> > > >
> > > > Please refer to this one. https://patchwork.kernel.org/patch/9645377/
> > >
> > > https://lkml.org/lkml/2017/3/27/57
> > >
> >
> > Ah, so this was reported before, and you sent the same fix. Well, it was
> > never
> > applied, so the bug is still there, and anyone who can use /dev/kvm can
> > trigger
> > it. So one of these patches needs to be applied, unless there is a better
> > fix.
> >
> > I don't agree with the "Fixes:" line in your version of the patch. The bug
> > was
> > actually there prior to 5419369ed, which might explain why that commit
> > seemed to
> > preserve the behavior intentionally. (Note that KVM_MEMORY_SLOTS did not
> > include the private memory slots; it was later renamed to
> > KVM_USER_MEM_SLOTS.)
> >
> > Eric
>
> Ping. Paolo or Radim, can you please consider applying one of these patches?
>
> Eric
Ping.
Re: [PATCH] KVM: prevent overlap between user and private memslots
On Fri, Jan 19, 2018 at 10:57:16AM -0800, Eric Biggers wrote:
> +Cc alex.william...@redhat.com
>
> On Fri, Jan 19, 2018 at 05:03:47PM +0800, Wanpeng Li wrote:
> > 2018-01-19 17:01 GMT+08:00 Wanpeng Li :
> > > 2018-01-19 16:18 GMT+08:00 Eric Biggers :
> > >> From: Eric Biggers
> > >>
> > >> 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: 7185ed42 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
> > >> #include
> > >> #include
> > >>
> > >> 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
> > >> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
> > >> Cc: # v2.6.25+
> > >> Signed-off-by: Eric Biggers
> > >
> > > Please refer to this one. https://patchwork.kernel.org/patch/9645377/
> >
> > https://lkml.org/lkml/2017/3/27/57
> >
>
> Ah, so this was reported before, and you sent the same fix. Well, it was
> never
> applied, so the bug is still there, and anyone who can use /dev/kvm can
> trigger
> it. So one of these patches needs to be applied, unless there is a better
> fix.
>
> I don't agree with the "Fixes:" line in your version of the patch. The bug
> was
> actually there prior to 5419369ed, which might explain why that commit seemed
> to
> preserve the behavior intentionally. (Note that KVM_MEMORY_SLOTS did not
> include the private memory slots; it was later renamed to KVM_USER_MEM_SLOTS.)
>
> Eric
Ping. Paolo or Radim, can you please consider applying one of these patches?
Eric
Re: [PATCH] KVM: prevent overlap between user and private memslots
+Cc alex.william...@redhat.com
On Fri, Jan 19, 2018 at 05:03:47PM +0800, Wanpeng Li wrote:
> 2018-01-19 17:01 GMT+08:00 Wanpeng Li :
> > 2018-01-19 16:18 GMT+08:00 Eric Biggers :
> >> From: Eric Biggers
> >>
> >> 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: 7185ed42 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
> >> #include
> >> #include
> >>
> >> 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
> >> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
> >> Cc: # v2.6.25+
> >> Signed-off-by: Eric Biggers
> >
> > Please refer to this one. https://patchwork.kernel.org/patch/9645377/
>
> https://lkml.org/lkml/2017/3/27/57
>
Ah, so this was reported before, and you sent the same fix. Well, it was never
applied, so the bug is still there, and anyone who can use /dev/kvm can trigger
it. So one of these patches needs to be applied, unless there is a better fix.
I don't agree with the "Fixes:" line in your version of the patch. The bug was
actually there prior to 5419369ed, which might explain why that commit seemed to
preserve the behavior intentionally. (Note that KVM_MEMORY_SLOTS did not
include the private memory slots; it was later renamed to KVM_USER_MEM_SLOTS.)
Eric
Re: [PATCH] KVM: prevent overlap between user and private memslots
2018-01-19 17:01 GMT+08:00 Wanpeng Li :
> 2018-01-19 16:18 GMT+08:00 Eric Biggers :
>> From: Eric Biggers
>>
>> 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: 7185ed42 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
>> #include
>> #include
>>
>> 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
>> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
>> Cc: # v2.6.25+
>> Signed-off-by: Eric Biggers
>
> 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
>>
Re: [PATCH] KVM: prevent overlap between user and private memslots
2018-01-19 16:18 GMT+08:00 Eric Biggers :
> From: Eric Biggers
>
> 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: 7185ed42 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
> #include
> #include
>
> 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
> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
> Cc: # v2.6.25+
> Signed-off-by: Eric Biggers
Please refer to this one. https://patchwork.kernel.org/patch/9645377/
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
>
