[libvirt] [PATCH 0/3] Make libvirt leak less memory

Ján Tomko (3): Free file header in virStorageFileGetMetadataRecurse Rework remoteSerializeDHCPLease Free DHCP leases file in networkGetDHCPLeasesHelper daemon/remote.c | 53 ++++++++++++++++++++++++++++---------------- src/network/bridge_driver.c | 1 + src/storage/storage_driver.c | 1 + 3 files changed, 36 insertions(+), 19 deletions(-) -- 1.8.5.5

Introduced by commit 2bdb8b9 --- src/storage/storage_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 7116185..a5adc63 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2906,6 +2906,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src, ret = 0; cleanup: + VIR_FREE(buf); virStorageFileDeinit(src); virStorageSourceFree(backingStore); return ret; -- 1.8.5.5

On Tue, Jun 24, 2014 at 02:45:32PM +0200, Ján Tomko wrote:
Introduced by commit 2bdb8b9 --- src/storage/storage_driver.c | 1 + 1 file changed, 1 insertion(+)
ACK 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 :|

Don't leak the temporary variables on success if NULL is returned for that field. Don't dereference NULL on failure to allocate some of the temporaries. Introduced by commit 990c3b6 --- daemon/remote.c | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index ae19b2a..9ffc1cb 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -6213,36 +6213,51 @@ remoteSerializeDHCPLease(remote_network_dhcp_lease *lease_dst, virNetworkDHCPLea char **hostname_tmp = NULL; char **clientid_tmp = NULL; - if (VIR_ALLOC(mac_tmp) < 0 || - VIR_ALLOC(iaid_tmp) < 0 || - VIR_ALLOC(hostname_tmp) < 0 || - VIR_ALLOC(clientid_tmp) < 0) - goto error; - lease_dst->expirytime = lease_src->expirytime; lease_dst->type = lease_src->type; lease_dst->prefix = lease_src->prefix; if (VIR_STRDUP(lease_dst->iface, lease_src->iface) < 0 || - VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0 || - VIR_STRDUP(*mac_tmp, lease_src->mac) < 0 || - VIR_STRDUP(*iaid_tmp, lease_src->iaid) < 0 || - VIR_STRDUP(*hostname_tmp, lease_src->hostname) < 0 || - VIR_STRDUP(*clientid_tmp, lease_src->clientid) < 0) + VIR_STRDUP(lease_dst->ipaddr, lease_src->ipaddr) < 0) goto error; - lease_dst->mac = *mac_tmp ? mac_tmp : NULL; - lease_dst->iaid = *iaid_tmp ? iaid_tmp : NULL; - lease_dst->hostname = *hostname_tmp ? hostname_tmp : NULL; - lease_dst->clientid = *clientid_tmp ? clientid_tmp : NULL; + if (lease_src->mac) { + if (VIR_ALLOC(mac_tmp) < 0 || + VIR_STRDUP(*mac_tmp, lease_src->mac) < 0) + goto error; + } + if (lease_src->iaid) { + if (VIR_ALLOC(iaid_tmp) < 0 || + VIR_STRDUP(*iaid_tmp, lease_src->iaid) < 0) + goto error; + } + if (lease_src->hostname) { + if (VIR_ALLOC(hostname_tmp) < 0 || + VIR_STRDUP(*hostname_tmp, lease_src->hostname) < 0) + goto error; + } + if (lease_src->clientid) { + if (VIR_ALLOC(clientid_tmp) < 0 || + VIR_STRDUP(*clientid_tmp, lease_src->clientid) < 0) + goto error; + } + + lease_dst->mac = mac_tmp; + lease_dst->iaid = iaid_tmp; + lease_dst->hostname = hostname_tmp; + lease_dst->clientid = clientid_tmp; return 0; error: - VIR_FREE(*mac_tmp); - VIR_FREE(*iaid_tmp); - VIR_FREE(*hostname_tmp); - VIR_FREE(*clientid_tmp); + if (mac_tmp) + VIR_FREE(*mac_tmp); + if (iaid_tmp) + VIR_FREE(*iaid_tmp); + if (hostname_tmp) + VIR_FREE(*hostname_tmp); + if (clientid_tmp) + VIR_FREE(*clientid_tmp); VIR_FREE(mac_tmp); VIR_FREE(iaid_tmp); VIR_FREE(hostname_tmp); -- 1.8.5.5

On Tue, Jun 24, 2014 at 02:45:33PM +0200, Ján Tomko wrote:
Don't leak the temporary variables on success if NULL is returned for that field.
Don't dereference NULL on failure to allocate some of the temporaries.
Introduced by commit 990c3b6 --- daemon/remote.c | 53 ++++++++++++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 19 deletions(-)
ACK 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 :|

Introduced by commit ba51398 --- src/network/bridge_driver.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index a407c6e..69c66f3 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -3529,6 +3529,7 @@ networkGetDHCPLeasesHelper(virNetworkObjPtr obj, cleanup: VIR_FREE(lease); + VIR_FREE(lease_entries); VIR_FREE(custom_lease_file); virJSONValueFree(leases_array); return rv; -- 1.8.5.5

On Tue, Jun 24, 2014 at 02:45:34PM +0200, Ján Tomko wrote:
Introduced by commit ba51398 --- src/network/bridge_driver.c | 1 + 1 file changed, 1 insertion(+)
ACK 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 :|

On 24.06.2014 14:45, Ján Tomko wrote:
Ján Tomko (3): Free file header in virStorageFileGetMetadataRecurse Rework remoteSerializeDHCPLease Free DHCP leases file in networkGetDHCPLeasesHelper
daemon/remote.c | 53 ++++++++++++++++++++++++++++---------------- src/network/bridge_driver.c | 1 + src/storage/storage_driver.c | 1 + 3 files changed, 36 insertions(+), 19 deletions(-)
ACK series Michal
participants (3)
-
Daniel P. Berrange
-
Ján Tomko
-
Michal Privoznik