lkml.org 
[lkml]   [2022]   [Dec]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
From
Subject答复: Found a memory leak in kvm module
Date
I had tried to retest with your patch,and I think the memory leak was resolved by it.
Thanks!

-----邮件原件-----
发件人: Sean Christopherson [mailto:seanjc@google.com]
发送时间: 2022年12月16日 2:17
收件人: 柳菁峰 <liujingfeng@qianxin.com>
抄送: linux-kernel@vger.kernel.org; kvm@vger.kernel.org; pbonzini@redhat.com; syzkaller@googlegroups.com
主题: Re: Found a memory leak in kvm module

On Mon, Dec 12, 2022, 柳菁峰 wrote:
> Hello,I have found a memory leak bug in kvm module by syzkaller.It was
> found in linux-5.4 but it also could be reproduced in the latest linux version.

Ah, I assume by "linux-5.4" you mean "stable v5.4.x kernels that contain commit
7d1bc32d6477 ("KVM: Stop looking for coalesced MMIO zones if the bus is destroyed")", because without that fix I can't see any bug that would affect both 5.4 and the upstream kernel.

If my assumption is correct, then I'm 99% certain the issue is that the target device isn't destroyed if allocating the new bus fails. I haven't had luck with the automatic fault injection, but was able to confirm a leak with this hack.

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 13e88297f999..22d9ab1b5c25 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -5424,7 +5424,7 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
struct kvm_io_device *dev) {
int i, j;
- struct kvm_io_bus *new_bus, *bus;
+ struct kvm_io_bus *new_bus = NULL, *bus;

lockdep_assert_held(&kvm->slots_lock);

@@ -5441,6 +5441,7 @@ int kvm_io_bus_unregister_dev(struct kvm *kvm, enum kvm_bus bus_idx,
if (i == bus->dev_count)
return 0;

+ if (!IS_ENABLED(CONFIG_X86_64))
new_bus = kmalloc(struct_size(bus, range, bus->dev_count - 1),
GFP_KERNEL_ACCOUNT);
if (new_bus) {

The fix is to destroy the target device before bailing. I'll send a proper patch either way, but it would be nice to get confirmation that this is the same bug that you hit with "linux-5.4".

Thanks!

---
virt/kvm/coalesced_mmio.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/virt/kvm/coalesced_mmio.c b/virt/kvm/coalesced_mmio.c index 0be80c213f7f..5ef88f5a0864 100644
--- a/virt/kvm/coalesced_mmio.c
+++ b/virt/kvm/coalesced_mmio.c
@@ -187,15 +187,17 @@ int kvm_vm_ioctl_unregister_coalesced_mmio(struct kvm *kvm,
r = kvm_io_bus_unregister_dev(kvm,
zone->pio ? KVM_PIO_BUS : KVM_MMIO_BUS, &dev->dev);

+ kvm_iodevice_destructor(&dev->dev);
+
/*
* On failure, unregister destroys all devices on the
* bus _except_ the target device, i.e. coalesced_zones
- * has been modified. No need to restart the walk as
- * there aren't any zones left.
+ * has been modified. Bail after destroying the target
+ * device, there's no need to restart the walk as there
+ * aren't any zones left.
*/
if (r)
break;
- kvm_iodevice_destructor(&dev->dev);
}
}

base-commit: 0f30b25edea48433eb32448990557364436818e6
--
\
 
 \ /
  Last update: 2022-12-19 08:05    [W:0.051 / U:0.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site