]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
daemon: avoid memleak when ListAll returns nothing
authorEric Blake <eblake@redhat.com>
Fri, 13 Mar 2015 23:01:43 +0000 (17:01 -0600)
committerEric Blake <eblake@redhat.com>
Mon, 16 Mar 2015 22:31:30 +0000 (16:31 -0600)
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

daemon/remote.c

index 89714ca422beb334bf951e3558bd6f9e2db04192..8ba678833fcddd23c6ff2f5858d288a890fca7c4 100644 (file)
@@ -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;
 }