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: [email protected]
Signed-off-by: Sabyrzhan Tasbolatov <[email protected]>
---
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);
--
2.25.1
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: [email protected]
> Signed-off-by: Sabyrzhan Tasbolatov <[email protected]>
> ---
> 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);
>