Message ID | 20211014120151.1437018-1-snovitoll@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | x86/kvm: restrict kvm user region memory size | expand |
On 14/10/21 14:01, Sabyrzhan Tasbolatov wrote: > syzbot found WARNING in memslot_rmap_alloc[1] when > struct kvm_userspace_memory_region .memory_size is bigger than > 0x40000000000, which is 4GB, e.g. KMALLOC_MAX_SIZE * 100 * PAGE_SIZE. > > Here is the PoC to trigger the warning: > > struct kvm_userspace_memory_region mem = { > .slot = 0, > .guest_phys_addr = 0, > /* + 0x100 extra to trigger kmalloc WARNING */ > .memory_size = 0x40000000000 + 0x100, > .userspace_addr = 0, > }; > > ioctl(kvm_fd, KVM_SET_USER_MEMORY_REGION, &mem); > > I couldn't find any relevant max constant to restrict unsigned long npages. > There might be another solution with chunking big portions of pages, but > there is already KVM_MAX_HUGEPAGE_LEVEL, though warning happens in > memslot_rmap_alloc() when level = 1, base_gfn = 0, e.g. > on the very first KVM_NR_PAGE_SIZES iteration. > > This is, seems, valid for early Linux versions as well. Can't tell which is > exactly can be considered for git bisect. > Here is Commit d89cc617b954af ("KVM: Push rmap into kvm_arch_memory_slot") > for example, Linux 3.7. The warning is bogus in this case. See the discussion in https://lkml.org/lkml/2021/9/7/669. The right fix is simply to use vmalloc instead of kmalloc. I'm woefully behind on my KVM maintainer duties, but this is on my todo list. Paolo > [1] > Call Trace: > kvmalloc include/linux/mm.h:806 [inline] > kvmalloc_array include/linux/mm.h:824 [inline] > kvcalloc include/linux/mm.h:829 [inline] > memslot_rmap_alloc+0xf6/0x310 arch/x86/kvm/x86.c:11320 > kvm_alloc_memslot_metadata arch/x86/kvm/x86.c:11388 [inline] > kvm_arch_prepare_memory_region+0x48d/0x610 arch/x86/kvm/x86.c:11462 > kvm_set_memslot+0xfe/0x1700 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1505 > ... > kvm_set_memory_region arch/x86/kvm/../../../virt/kvm/kvm_main.c:1689 > kvm_vm_ioctl_set_memory_region arch/x86/kvm/../../../virt/kvm/kvm_main.c > > Reported-by: syzbot+e0de2333cbf95ea473e8@syzkaller.appspotmail.com > Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com> > --- > arch/x86/kvm/mmu/page_track.c | 3 +++ > arch/x86/kvm/x86.c | 3 +++ > 2 files changed, 6 insertions(+) > > diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c > index 21427e84a82e..e790bb341680 100644 > --- a/arch/x86/kvm/mmu/page_track.c > +++ b/arch/x86/kvm/mmu/page_track.c > @@ -35,6 +35,9 @@ int kvm_page_track_create_memslot(struct kvm_memory_slot *slot, > int i; > > for (i = 0; i < KVM_PAGE_TRACK_MAX; i++) { > + if (npages > KMALLOC_MAX_SIZE) > + return -ENOMEM; > + > slot->arch.gfn_track[i] = > kvcalloc(npages, sizeof(*slot->arch.gfn_track[i]), > GFP_KERNEL_ACCOUNT); > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index aabd3a2ec1bc..2bad607976a9 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -11394,6 +11394,9 @@ static int memslot_rmap_alloc(struct kvm_memory_slot *slot, > > WARN_ON(slot->arch.rmap[i]); > > + if (lpages > KMALLOC_MAX_SIZE) > + return -ENOMEM; > + > slot->arch.rmap[i] = kvcalloc(lpages, sz, GFP_KERNEL_ACCOUNT); > if (!slot->arch.rmap[i]) { > memslot_rmap_free(slot); >
diff --git a/arch/x86/kvm/mmu/page_track.c b/arch/x86/kvm/mmu/page_track.c index 21427e84a82e..e790bb341680 100644 --- a/arch/x86/kvm/mmu/page_track.c +++ b/arch/x86/kvm/mmu/page_track.c @@ -35,6 +35,9 @@ int kvm_page_track_create_memslot(struct kvm_memory_slot *slot, int i; for (i = 0; i < KVM_PAGE_TRACK_MAX; i++) { + if (npages > KMALLOC_MAX_SIZE) + return -ENOMEM; + slot->arch.gfn_track[i] = kvcalloc(npages, sizeof(*slot->arch.gfn_track[i]), GFP_KERNEL_ACCOUNT); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index aabd3a2ec1bc..2bad607976a9 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -11394,6 +11394,9 @@ static int memslot_rmap_alloc(struct kvm_memory_slot *slot, WARN_ON(slot->arch.rmap[i]); + if (lpages > KMALLOC_MAX_SIZE) + return -ENOMEM; + slot->arch.rmap[i] = kvcalloc(lpages, sz, GFP_KERNEL_ACCOUNT); if (!slot->arch.rmap[i]) { memslot_rmap_free(slot);
syzbot found WARNING in memslot_rmap_alloc[1] when struct kvm_userspace_memory_region .memory_size is bigger than 0x40000000000, which is 4GB, e.g. KMALLOC_MAX_SIZE * 100 * PAGE_SIZE. Here is the PoC to trigger the warning: struct kvm_userspace_memory_region mem = { .slot = 0, .guest_phys_addr = 0, /* + 0x100 extra to trigger kmalloc WARNING */ .memory_size = 0x40000000000 + 0x100, .userspace_addr = 0, }; ioctl(kvm_fd, KVM_SET_USER_MEMORY_REGION, &mem); I couldn't find any relevant max constant to restrict unsigned long npages. There might be another solution with chunking big portions of pages, but there is already KVM_MAX_HUGEPAGE_LEVEL, though warning happens in memslot_rmap_alloc() when level = 1, base_gfn = 0, e.g. on the very first KVM_NR_PAGE_SIZES iteration. This is, seems, valid for early Linux versions as well. Can't tell which is exactly can be considered for git bisect. Here is Commit d89cc617b954af ("KVM: Push rmap into kvm_arch_memory_slot") for example, Linux 3.7. [1] Call Trace: kvmalloc include/linux/mm.h:806 [inline] kvmalloc_array include/linux/mm.h:824 [inline] kvcalloc include/linux/mm.h:829 [inline] memslot_rmap_alloc+0xf6/0x310 arch/x86/kvm/x86.c:11320 kvm_alloc_memslot_metadata arch/x86/kvm/x86.c:11388 [inline] kvm_arch_prepare_memory_region+0x48d/0x610 arch/x86/kvm/x86.c:11462 kvm_set_memslot+0xfe/0x1700 arch/x86/kvm/../../../virt/kvm/kvm_main.c:1505 ... kvm_set_memory_region arch/x86/kvm/../../../virt/kvm/kvm_main.c:1689 kvm_vm_ioctl_set_memory_region arch/x86/kvm/../../../virt/kvm/kvm_main.c Reported-by: syzbot+e0de2333cbf95ea473e8@syzkaller.appspotmail.com Signed-off-by: Sabyrzhan Tasbolatov <snovitoll@gmail.com> --- arch/x86/kvm/mmu/page_track.c | 3 +++ arch/x86/kvm/x86.c | 3 +++ 2 files changed, 6 insertions(+)