[libvirt] [PATCH 2/4] src: fix multiple resource leaks in loops

Pavel Hrdina posted 4 patches 8 years, 11 months ago
[libvirt] [PATCH 2/4] src: fix multiple resource leaks in loops
Posted by Pavel Hrdina 8 years, 11 months ago
All of the variables are filled inside a loop and therefore
needs to be also freed in every cycle.

Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
---
 src/conf/node_device_conf.c | 10 +++++-----
 src/conf/storage_conf.c     |  1 +
 src/util/virsysinfo.c       |  2 ++
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
index 7d0baa9d1a..e21a98d51f 100644
--- a/src/conf/node_device_conf.c
+++ b/src/conf/node_device_conf.c
@@ -1582,7 +1582,6 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
     for (i = 0, m = 0; i < n; i++) {
         xmlNodePtr node = nodes[i];
         char *tmp = virXMLPropString(node, "type");
-        virNodeDevDevnodeType type;
         int val;
 
         if (!tmp) {
@@ -1591,15 +1590,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
             goto error;
         }
 
-        if ((val = virNodeDevDevnodeTypeFromString(tmp)) < 0) {
+        val = virNodeDevDevnodeTypeFromString(tmp);
+        VIR_FREE(tmp);
+
+        if (val < 0) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            _("unknown devnode type '%s'"), tmp);
-            VIR_FREE(tmp);
             goto error;
         }
-        type = val;
 
-        switch (type) {
+        switch ((virNodeDevDevnodeType)val) {
         case VIR_NODE_DEV_DEVNODE_DEV:
             def->devnode = (char*)xmlNodeGetContent(node);
             break;
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index fe0f0bcdc7..b66e67254a 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -471,6 +471,7 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
                                    port);
                     goto cleanup;
                 }
+                VIR_FREE(port);
             }
         }
     }
diff --git a/src/util/virsysinfo.c b/src/util/virsysinfo.c
index 8d3377c04e..650216d006 100644
--- a/src/util/virsysinfo.c
+++ b/src/util/virsysinfo.c
@@ -521,6 +521,8 @@ virSysinfoParseS390Processor(const char *base, virSysinfoDefPtr ret)
                                           &processor->processor_family,
                                           '=', '\n'))
             goto cleanup;
+
+        VIR_FREE(procline);
     }
     result = 0;
 
-- 
2.12.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] src: fix multiple resource leaks in loops
Posted by Ján Tomko 8 years, 11 months ago
On Mon, Apr 10, 2017 at 09:53:58AM +0200, Pavel Hrdina wrote:
>All of the variables are filled inside a loop and therefore
>needs to be also freed in every cycle.
>
>Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
>---
> src/conf/node_device_conf.c | 10 +++++-----
> src/conf/storage_conf.c     |  1 +
> src/util/virsysinfo.c       |  2 ++
> 3 files changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c
>index 7d0baa9d1a..e21a98d51f 100644
>--- a/src/conf/node_device_conf.c
>+++ b/src/conf/node_device_conf.c
>@@ -1582,7 +1582,6 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
>     for (i = 0, m = 0; i < n; i++) {
>         xmlNodePtr node = nodes[i];
>         char *tmp = virXMLPropString(node, "type");
>-        virNodeDevDevnodeType type;
>         int val;
>
>         if (!tmp) {
>@@ -1591,15 +1590,16 @@ virNodeDeviceDefParseXML(xmlXPathContextPtr ctxt,
>             goto error;
>         }
>
>-        if ((val = virNodeDevDevnodeTypeFromString(tmp)) < 0) {
>+        val = virNodeDevDevnodeTypeFromString(tmp);
>+        VIR_FREE(tmp);
>+
>+        if (val < 0) {
>             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>                            _("unknown devnode type '%s'"), tmp);

This accesses the 'tmp' variable after it has been freed.

I suggest preserving the error message, so there will be two VIR_FREE's
needed - one for the success path and one for the error path.

Jan

>-            VIR_FREE(tmp);
>             goto error;
>         }
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list