[libvirt] [PATCH 0/4] Plug some virsh leaks
https://bugzilla.redhat.com/show_bug.cgi?id=1001536 Ján Tomko (4): virsh: free messages after logging them to a file virsh: free the list from ListAll APIs even for 0 items virsh: free the formatting string when listing pool details virsh: free the caps list properly if one of them is invalid tools/virsh-interface.c | 2 +- tools/virsh-network.c | 2 +- tools/virsh-nodedev.c | 6 +++--- tools/virsh-nwfilter.c | 2 +- tools/virsh-pool.c | 3 ++- tools/virsh-secret.c | 2 +- tools/virsh.c | 1 + 7 files changed, 10 insertions(+), 8 deletions(-) -- 1.8.1.5
The messages were only freed on error. ==12== 1,100 bytes in 1 blocks are definitely lost in loss record 698 of 729 ==12== by 0x4E98C22: virBufferAsprintf (virbuffer.c:294) ==12== by 0x12C950: vshOutputLogFile (virsh.c:2440) ==12== by 0x12880B: vshError (virsh.c:2254) ==12== by 0x131957: vshCommandOptDomainBy (virsh-domain.c:109) ==12== by 0x14253E: cmdStart (virsh-domain.c:3333) https://bugzilla.redhat.com/show_bug.cgi?id=1001536 --- tools/virsh.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/virsh.c b/tools/virsh.c index 2ea44a6..dfd3665 100644 --- a/tools/virsh.c +++ b/tools/virsh.c @@ -2476,6 +2476,7 @@ vshOutputLogFile(vshControl *ctl, int log_level, const char *msg_format, if (safewrite(ctl->log_fd, str, len) < 0) goto error; + VIR_FREE(str); return; error: -- 1.8.1.5
virsh secret-list leak when no secrets are defined: ==27== 8 bytes in 1 blocks are definitely lost in loss record 6 of 726 ==27== by 0x4E941DD: virAllocN (viralloc.c:183) ==27== by 0x5037F1A: remoteConnectListAllSecrets (remote_driver.c:3076) ==27== by 0x5004EC6: virConnectListAllSecrets (libvirt.c:16298) ==27== by 0x15F813: vshSecretListCollect (virsh-secret.c:397) ==27== by 0x15F0E1: cmdSecretList (virsh-secret.c:532) And so do some other *-list commands. https://bugzilla.redhat.com/show_bug.cgi?id=1001536 --- tools/virsh-interface.c | 2 +- tools/virsh-network.c | 2 +- tools/virsh-nodedev.c | 2 +- tools/virsh-nwfilter.c | 2 +- tools/virsh-secret.c | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/virsh-interface.c b/tools/virsh-interface.c index 0f78551..3720c8c 100644 --- a/tools/virsh-interface.c +++ b/tools/virsh-interface.c @@ -174,7 +174,7 @@ vshInterfaceListFree(vshInterfaceListPtr list) { size_t i; - if (list && list->nifaces) { + if (list && list->ifaces) { for (i = 0; i < list->nifaces; i++) { if (list->ifaces[i]) virInterfaceFree(list->ifaces[i]); diff --git a/tools/virsh-network.c b/tools/virsh-network.c index 06bf483..8ddd5ca 100644 --- a/tools/virsh-network.c +++ b/tools/virsh-network.c @@ -423,7 +423,7 @@ vshNetworkListFree(vshNetworkListPtr list) { size_t i; - if (list && list->nnets) { + if (list && list->nets) { for (i = 0; i < list->nnets; i++) { if (list->nets[i]) virNetworkFree(list->nets[i]); diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index 5522405..b34a3b1 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -207,7 +207,7 @@ vshNodeDeviceListFree(vshNodeDeviceListPtr list) { size_t i; - if (list && list->ndevices) { + if (list && list->devices) { for (i = 0; i < list->ndevices; i++) { if (list->devices[i]) virNodeDeviceFree(list->devices[i]); diff --git a/tools/virsh-nwfilter.c b/tools/virsh-nwfilter.c index fbf211a..e5fcb8f 100644 --- a/tools/virsh-nwfilter.c +++ b/tools/virsh-nwfilter.c @@ -242,7 +242,7 @@ vshNWFilterListFree(vshNWFilterListPtr list) { size_t i; - if (list && list->nfilters) { + if (list && list->filters) { for (i = 0; i < list->nfilters; i++) { if (list->filters[i]) virNWFilterFree(list->filters[i]); diff --git a/tools/virsh-secret.c b/tools/virsh-secret.c index ac99840..e849a79 100644 --- a/tools/virsh-secret.c +++ b/tools/virsh-secret.c @@ -370,7 +370,7 @@ vshSecretListFree(vshSecretListPtr list) { size_t i; - if (list && list->nsecrets) { + if (list && list->secrets) { for (i = 0; i < list->nsecrets; i++) { if (list->secrets[i]) virSecretFree(list->secrets[i]); -- 1.8.1.5
==23== 41 bytes in 1 blocks are definitely lost in loss record 626 of 727 ==23== by 0x4F0099F: virAsprintfInternal (virstring.c:358) ==23== by 0x15D2C9: cmdPoolList (virsh-pool.c:1268) https://bugzilla.redhat.com/show_bug.cgi?id=1001536 --- tools/virsh-pool.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/virsh-pool.c b/tools/virsh-pool.c index 592b81f..3d3c61e 100644 --- a/tools/virsh-pool.c +++ b/tools/virsh-pool.c @@ -1264,7 +1264,7 @@ cmdPoolList(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) /* Create the output template. Each column is sized according to * the longest string. */ - char *outputStr; + char *outputStr = NULL; ret = virAsprintf(&outputStr, "%%-%lus %%-%lus %%-%lus %%-%lus %%%lus %%%lus %%%lus\n", (unsigned long) nameStrLength, @@ -1319,6 +1319,7 @@ asprintf_failure: functionReturn = false; cleanup: + VIR_FREE(outputStr); if (list && list->npools) { for (i = 0; i < list->npools; i++) { VIR_FREE(poolInfoTexts[i].state); -- 1.8.1.5
VIR_FREE(caps) is not enough to free an array allocated by vshStringToArray. ==17== 4 bytes in 1 blocks are definitely lost in loss record 4 of 728 ==17== by 0x4EFFC44: virStrdup (virstring.c:554) ==17== by 0x128B10: _vshStrdup (virsh.c:125) ==17== by 0x129164: vshStringToArray (virsh.c:218) ==17== by 0x157BB3: cmdNodeListDevices (virsh-nodedev.c:409) https://bugzilla.redhat.com/show_bug.cgi?id=1001536 --- tools/virsh-nodedev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/virsh-nodedev.c b/tools/virsh-nodedev.c index b34a3b1..3a2d12b 100644 --- a/tools/virsh-nodedev.c +++ b/tools/virsh-nodedev.c @@ -413,8 +413,8 @@ cmdNodeListDevices(vshControl *ctl, const vshCmd *cmd ATTRIBUTE_UNUSED) for (i = 0; i < ncaps; i++) { if ((cap_type = virNodeDevCapTypeFromString(caps[i])) < 0) { vshError(ctl, "%s", _("Invalid capability type")); - VIR_FREE(caps); - return false; + ret = false; + goto cleanup; } switch (cap_type) { -- 1.8.1.5
On 08/27/2013 07:06 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
Ján Tomko (4): virsh: free messages after logging them to a file virsh: free the list from ListAll APIs even for 0 items virsh: free the formatting string when listing pool details virsh: free the caps list properly if one of them is invalid
ACK series.
tools/virsh-interface.c | 2 +- tools/virsh-network.c | 2 +- tools/virsh-nodedev.c | 6 +++--- tools/virsh-nwfilter.c | 2 +- tools/virsh-pool.c | 3 ++- tools/virsh-secret.c | 2 +- tools/virsh.c | 1 + 7 files changed, 10 insertions(+), 8 deletions(-)
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 08/28/2013 03:41 AM, Eric Blake wrote:
On 08/27/2013 07:06 AM, Ján Tomko wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1001536
Ján Tomko (4): virsh: free messages after logging them to a file virsh: free the list from ListAll APIs even for 0 items virsh: free the formatting string when listing pool details virsh: free the caps list properly if one of them is invalid
ACK series.
Thanks; pushed now. Jan
participants (2)
-
Eric Blake -
Ján Tomko