[libvirt] [PATCH v2] Fix Memory Leak in virQEMUCapsInitGuestFromBinary()
While running qemucaps2xmltest, it was found that valgrind pointed out the following memory leaks: ==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65 ==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593) ==29896== by 0x4C6B45E: virAllocN (viralloc.c:191) ==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999) ==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789) ==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118) ==29896== by 0x41FFD1: virtTestRun (testutils.c:201) ==29896== by 0x41EE7A: mymain (qemucaps2xmltest.c:203) ==29896== by 0x42074D: virtTestMain (testutils.c:789) ==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ==29896== ==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65 ==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593) ==29896== by 0x4C6B45E: virAllocN (viralloc.c:191) ==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999) ==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789) ==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118) ==29896== by 0x41FFD1: virtTestRun (testutils.c:201) ==29896== by 0x41EEA3: mymain (qemucaps2xmltest.c:204) ==29896== by 0x42074D: virtTestMain (testutils.c:789) ==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) --- src/qemu/qemu_capabilities.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7673592..aef8bc1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -789,6 +789,10 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (virQEMUCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) goto cleanup; + /* Free unneeded memory given by malloc(0) */ + if (!nmachines) + VIR_FREE(machines); + /* We register kvm as the base emulator too, since we can * just give -no-kvm to disable acceleration if required */ if ((guest = virCapabilitiesAddGuest(caps, -- 1.7.1
On Thu, Mar 27, 2014 at 03:17:09AM +0530, Nehal J Wani wrote:
While running qemucaps2xmltest, it was found that valgrind pointed out the following memory leaks:
==29896== 0 bytes in 1 blocks are definitely lost in loss record 1 of 65 ==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593) ==29896== by 0x4C6B45E: virAllocN (viralloc.c:191) ==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999) ==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789) ==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118) ==29896== by 0x41FFD1: virtTestRun (testutils.c:201) ==29896== by 0x41EE7A: mymain (qemucaps2xmltest.c:203) ==29896== by 0x42074D: virtTestMain (testutils.c:789) ==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226) ==29896== ==29896== 0 bytes in 1 blocks are definitely lost in loss record 2 of 65 ==29896== at 0x4A0577B: calloc (vg_replace_malloc.c:593) ==29896== by 0x4C6B45E: virAllocN (viralloc.c:191) ==29896== by 0x4232A9: virQEMUCapsGetMachineTypesCaps (qemu_capabilities.c:1999) ==29896== by 0x4234E7: virQEMUCapsInitGuestFromBinary (qemu_capabilities.c:789) ==29896== by 0x41F10B: testQemuCapsXML (qemucaps2xmltest.c:118) ==29896== by 0x41FFD1: virtTestRun (testutils.c:201) ==29896== by 0x41EEA3: mymain (qemucaps2xmltest.c:204) ==29896== by 0x42074D: virtTestMain (testutils.c:789) ==29896== by 0x3E6CE1ED1C: (below main) (libc-start.c:226)
--- src/qemu/qemu_capabilities.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 7673592..aef8bc1 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -789,6 +789,10 @@ virQEMUCapsInitGuestFromBinary(virCapsPtr caps, if (virQEMUCapsGetMachineTypesCaps(qemubinCaps, &nmachines, &machines) < 0) goto cleanup;
+ /* Free unneeded memory given by malloc(0) */ + if (!nmachines) + VIR_FREE(machines); + /* We register kvm as the base emulator too, since we can * just give -no-kvm to disable acceleration if required */ if ((guest = virCapabilitiesAddGuest(caps,
This doesn't look at all right either. If nmachines is 0, then machines should already be NULL. Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
This doesn't look at all right either. If nmachines is 0, then machines should already be NULL.
If you look at the code of virQEMUCapsGetMachineTypesCaps, you see: int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, size_t *nmachines, virCapsGuestMachinePtr **machines) { size_t i; *nmachines = 0; *machines = NULL; if (VIR_ALLOC_N(*machines, qemuCaps->nmachineTypes) < 0) goto error; *nmachines = qemuCaps->nmachineTypes; Even if we pass nmachines=0 to VIR_ALLOC_N , it emulates GNU behavior of malloc(0) allocating a pointer, which is never freed, and hence needs a VIR_FREE. -- Nehal J Wani
On Thu, Mar 27, 2014 at 04:39:28PM +0530, Nehal J Wani wrote:
This doesn't look at all right either. If nmachines is 0, then machines should already be NULL.
If you look at the code of virQEMUCapsGetMachineTypesCaps, you see: int virQEMUCapsGetMachineTypesCaps(virQEMUCapsPtr qemuCaps, size_t *nmachines, virCapsGuestMachinePtr **machines) { size_t i;
*nmachines = 0; *machines = NULL; if (VIR_ALLOC_N(*machines, qemuCaps->nmachineTypes) < 0) goto error; *nmachines = qemuCaps->nmachineTypes;
Even if we pass nmachines=0 to VIR_ALLOC_N , it emulates GNU behavior of malloc(0) allocating a pointer, which is never freed, and hence needs a VIR_FREE.
IMHO we should fix that code so it doesn't allocate the machines array. ie we shouldn't require callers to free data that they're not expecting to be allocated in the first place Regards, Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
participants (2)
-
Daniel P. Berrange -
Nehal J Wani