[PATCH] network: Remove memory leak caused by wrong initialization

Julio Faracco jcfaracco at gmail.com
Sat Apr 25 16:35:37 UTC 2020


This commit fix a wrong variable initialization. There is a variable
called `new_lease` which is being initialized with the content of
parameter `lease`. To avoid memory leak, the proper way is initialize
with NULL first. This wrong statement was added by commit 97a0aa24.
There are some other improvements also.

Signed-off-by: Julio Faracco <jcfaracco at gmail.com>
---
 src/conf/network_conf.c     | 38 ++++++++++++++++++-------------------
 src/network/bridge_driver.c |  8 ++------
 2 files changed, 21 insertions(+), 25 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index cd60ee7548..a7c177f8db 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -412,38 +412,32 @@ static int
 virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease,
                                    xmlNodePtr node)
 {
-    virNetworkDHCPLeaseTimeDefPtr new_lease = *lease;
-    g_autofree char *expiry = NULL;
-    g_autofree char *unit = NULL;
-    int unitInt;
+    virNetworkDHCPLeaseTimeDefPtr new_lease = NULL;
+    g_autofree char *expirystr = NULL;
+    g_autofree char *unitstr = NULL;
+    unsigned long expiry;
+    int unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES;
 
-    if (!(expiry = virXMLPropString(node, "expiry")))
+    if (!(expirystr = virXMLPropString(node, "expiry")))
         return 0;
 
-    if (VIR_ALLOC(new_lease) < 0)
-        return -1;
-    new_lease->unit = VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES;
-
-    if (virStrToLong_ul(expiry, NULL, 10, &new_lease->expiry) < 0)
+    if (virStrToLong_ul(expirystr, NULL, 10, &expiry) < 0)
         return -1;
 
-    if ((unit = virXMLPropString(node, "unit"))) {
-        if ((unitInt = virNetworkDHCPLeaseTimeUnitTypeFromString(unit)) < 0) {
+    if ((unitstr = virXMLPropString(node, "unit"))) {
+        if ((unit = virNetworkDHCPLeaseTimeUnitTypeFromString(unitstr)) < 0) {
             virReportError(VIR_ERR_XML_ERROR,
-                           _("Invalid unit: %s"), unit);
+                           _("Invalid unit: %s"), unitstr);
             return -1;
         }
-        new_lease->unit = unitInt;
     }
 
     /* infinite */
-    if (new_lease->expiry > 0) {
+    if (expiry > 0) {
         /* This boundary check is related to dnsmasq man page settings:
          * "The minimum lease time is two minutes." */
-        if ((new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS &&
-             new_lease->expiry < 120) ||
-            (new_lease->unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES &&
-             new_lease->expiry < 2)) {
+        if ((unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_SECONDS && expiry < 120) ||
+            (unit == VIR_NETWORK_DHCP_LEASETIME_UNIT_MINUTES && expiry < 2)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
                            _("The minimum lease time should be greater "
                              "than 2 minutes"));
@@ -451,6 +445,12 @@ virNetworkDHCPLeaseTimeDefParseXML(virNetworkDHCPLeaseTimeDefPtr *lease,
         }
     }
 
+    if (VIR_ALLOC(new_lease) < 0)
+        return -1;
+
+    new_lease->expiry = expiry;
+    new_lease->unit = unit;
+
     *lease = new_lease;
 
     return 0;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 87f0452611..e8f0dcf7d0 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -969,7 +969,6 @@ static int networkConnectIsAlive(virConnectPtr conn G_GNUC_UNUSED)
 static char *
 networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease)
 {
-    char *leasetime = NULL;
     const char *unit;
     g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
 
@@ -984,9 +983,7 @@ networkBuildDnsmasqLeaseTime(virNetworkDHCPLeaseTimeDefPtr lease)
         virBufferAsprintf(&buf, "%lu%c", lease->expiry, unit[0]);
     }
 
-    leasetime = virBufferContentAndReset(&buf);
-
-    return leasetime;
+    return virBufferContentAndReset(&buf);
 }
 
 
@@ -999,14 +996,13 @@ networkBuildDnsmasqDhcpHostsList(dnsmasqContext *dctx,
 {
     size_t i;
     bool ipv6 = false;
-    g_autofree char *leasetime = NULL;
 
     if (VIR_SOCKET_ADDR_IS_FAMILY(&ipdef->address, AF_INET6))
         ipv6 = true;
     for (i = 0; i < ipdef->nhosts; i++) {
         virNetworkDHCPHostDefPtr host = &(ipdef->hosts[i]);
+        g_autofree char *leasetime = networkBuildDnsmasqLeaseTime(host->lease);
 
-        leasetime = networkBuildDnsmasqLeaseTime(host->lease);
         if (VIR_SOCKET_ADDR_VALID(&host->ip))
             if (dnsmasqAddDhcpHost(dctx, host->mac, &host->ip,
                                    host->name, host->id, leasetime,
-- 
2.25.3





More information about the libvir-list mailing list