2018-01-19 16:18 GMT+08:00 Eric Biggers <ebiggers3@xxxxxxxxx>:
> From: Eric Biggers <ebiggers@xxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxxx>
> Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots")
> Cc: <stable@xxxxxxxxxxxxxxx> # v2.6.25+
> Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
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
>
