summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Blake <eblake@redhat.com>2015-03-13 17:01:43 -0600
committerEric Blake <eblake@redhat.com>2015-03-16 16:31:30 -0600
commit4fc4f669eb6a1d776b917d410b6db46e09b6feed (patch)
tree514178aab4ab59a631269127356cab2365d1c6eb
parent394b31f0e38d44bee315404c2a8d5f233fb99c42 (diff)
downloadlibvirt-4fc4f669eb6a1d776b917d410b6db46e09b6feed.tar.gz
daemon: avoid memleak when ListAll returns nothing
Commit 4f25146 (v1.2.8) managed to silence Coverity, but at the cost of a memory leak detected by valgrind: ==24129== 40 bytes in 5 blocks are definitely lost in loss record 355 of 637 ==24129== at 0x4A08B1C: realloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==24129== by 0x5084B8E: virReallocN (viralloc.c:245) ==24129== by 0x514D5AA: virDomainObjListExport (domain_conf.c:22200) ==24129== by 0x201227DB: qemuConnectListAllDomains (qemu_driver.c:18042) ==24129== by 0x51CC1B6: virConnectListAllDomains (libvirt-domain.c:6797) ==24129== by 0x14173D: remoteDispatchConnectListAllDomains (remote.c:1580) ==24129== by 0x121BE1: remoteDispatchConnectListAllDomainsHelper (remote_dispatch.h:1072) In short, every time a client calls a ListAll variant and asks for the resulting list, but there are 0 elements to return, we end up leaking the 1-entry array that holds the NULL terminator. What's worse, a read-only client can access these functions in a tight loop to cause libvirtd to eventually run out of memory; and this can be considered a denial of service attack against more privileged clients. Thankfully, the leak is so small (8 bytes per call) that you would already have some other denial of service with any guest calling the API that frequently, so an out-of-memory crash is unlikely enough that this did not warrant a CVE. * daemon/remote.c (remoteDispatchConnectListAllDomains) (remoteDispatchDomainListAllSnapshots) (remoteDispatchDomainSnapshotListAllChildren) (remoteDispatchConnectListAllStoragePools) (remoteDispatchStoragePoolListAllVolumes) (remoteDispatchConnectListAllNetworks) (remoteDispatchConnectListAllInterfaces) (remoteDispatchConnectListAllNodeDevices) (remoteDispatchConnectListAllNWFilters) (remoteDispatchConnectListAllSecrets) (remoteDispatchNetworkGetDHCPLeases): Plug leak. Signed-off-by: Eric Blake <eblake@redhat.com> (cherry picked from commit 3c2ff5029b83c9b33be0f1607a3c61f4f5850612) Signed-off-by: Eric Blake <eblake@redhat.com> Conflicts: daemon/remote.c - context with older cleanup styles
-rw-r--r--daemon/remote.c55
1 files changed, 22 insertions, 33 deletions
diff --git a/daemon/remote.c b/daemon/remote.c
index 89714ca422..8ba678833f 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -1535,11 +1535,10 @@ remoteDispatchConnectListAllDomains(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (doms && ndomains > 0) {
+ if (doms && ndomains > 0)
for (i = 0; i < ndomains; i++)
virDomainFree(doms[i]);
- VIR_FREE(doms);
- }
+ VIR_FREE(doms);
return rv;
}
@@ -4698,11 +4697,10 @@ remoteDispatchDomainListAllSnapshots(virNetServerPtr server ATTRIBUTE_UNUSED,
virNetMessageSaveError(rerr);
if (dom)
virDomainFree(dom);
- if (snaps && nsnaps > 0) {
+ if (snaps && nsnaps > 0)
for (i = 0; i < nsnaps; i++)
virDomainSnapshotFree(snaps[i]);
- VIR_FREE(snaps);
- }
+ VIR_FREE(snaps);
return rv;
}
@@ -4769,11 +4767,10 @@ remoteDispatchDomainSnapshotListAllChildren(virNetServerPtr server ATTRIBUTE_UNU
virDomainSnapshotFree(snapshot);
if (dom)
virDomainFree(dom);
- if (snaps && nsnaps > 0) {
+ if (snaps && nsnaps > 0)
for (i = 0; i < nsnaps; i++)
virDomainSnapshotFree(snaps[i]);
- VIR_FREE(snaps);
- }
+ VIR_FREE(snaps);
return rv;
}
@@ -4828,11 +4825,10 @@ remoteDispatchConnectListAllStoragePools(virNetServerPtr server ATTRIBUTE_UNUSED
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (pools && npools > 0) {
+ if (pools && npools > 0)
for (i = 0; i < npools; i++)
virStoragePoolFree(pools[i]);
- VIR_FREE(pools);
- }
+ VIR_FREE(pools);
return rv;
}
@@ -4891,11 +4887,10 @@ remoteDispatchStoragePoolListAllVolumes(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (vols && nvols > 0) {
+ if (vols && nvols > 0)
for (i = 0; i < nvols; i++)
virStorageVolFree(vols[i]);
- VIR_FREE(vols);
- }
+ VIR_FREE(vols);
if (pool)
virStoragePoolFree(pool);
return rv;
@@ -4952,11 +4947,10 @@ remoteDispatchConnectListAllNetworks(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (nets && nnets > 0) {
+ if (nets && nnets > 0)
for (i = 0; i < nnets; i++)
virNetworkFree(nets[i]);
- VIR_FREE(nets);
- }
+ VIR_FREE(nets);
return rv;
}
@@ -5011,11 +5005,10 @@ remoteDispatchConnectListAllInterfaces(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (ifaces && nifaces > 0) {
+ if (ifaces && nifaces > 0)
for (i = 0; i < nifaces; i++)
virInterfaceFree(ifaces[i]);
- VIR_FREE(ifaces);
- }
+ VIR_FREE(ifaces);
return rv;
}
@@ -5070,11 +5063,10 @@ remoteDispatchConnectListAllNodeDevices(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (devices && ndevices > 0) {
+ if (devices && ndevices > 0)
for (i = 0; i < ndevices; i++)
virNodeDeviceFree(devices[i]);
- VIR_FREE(devices);
- }
+ VIR_FREE(devices);
return rv;
}
@@ -5129,11 +5121,10 @@ remoteDispatchConnectListAllNWFilters(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (filters && nfilters > 0) {
+ if (filters && nfilters > 0)
for (i = 0; i < nfilters; i++)
virNWFilterFree(filters[i]);
- VIR_FREE(filters);
- }
+ VIR_FREE(filters);
return rv;
}
@@ -5188,11 +5179,10 @@ remoteDispatchConnectListAllSecrets(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (secrets && nsecrets > 0) {
+ if (secrets && nsecrets > 0)
for (i = 0; i < nsecrets; i++)
virSecretFree(secrets[i]);
- VIR_FREE(secrets);
- }
+ VIR_FREE(secrets);
return rv;
}
@@ -6373,11 +6363,10 @@ remoteDispatchNetworkGetDHCPLeases(virNetServerPtr server ATTRIBUTE_UNUSED,
cleanup:
if (rv < 0)
virNetMessageSaveError(rerr);
- if (leases && nleases > 0) {
+ if (leases && nleases > 0)
for (i = 0; i < nleases; i++)
virNetworkDHCPLeaseFree(leases[i]);
- VIR_FREE(leases);
- }
+ VIR_FREE(leases);
virNetworkFree(net);
return rv;
}