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/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@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 --- 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)))