On Fri, Jan 19, 2018 at 10:57:16AM -0800, Eric Biggers wrote:
+Cc alex.williamson@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 kernellwp@gmail.com:
2018-01-19 16:18 GMT+08:00 Eric Biggers ebiggers3@gmail.com:
From: Eric Biggers ebiggers@google.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/0x8bReproducer:
#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@googlegroups.com Fixes: e0d62c7f4860 ("KVM: Add kernel-internal memory slots") Cc: stable@vger.kernel.org # v2.6.25+ Signed-off-by: Eric Biggers ebiggers@google.com
Please refer to this one. https://patchwork.kernel.org/patch/9645377/
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