From a55eaced607c9253f8422e2a207ef12039f99558 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Fri, 29 Sep 2017 15:55:29 -0400
Subject: [PATCH] nwfilter: Don't have virNWFilterIPAddrMapAddIPAddr consume
 input

On pure success paths, virNWFilterIPAddrMapAddIPAddr was validly
consuming the input @addr; however, on failure paths it was possible
that virNWFilterVarValueCreateSimple succeed, but virNWFilterHashTablePut
failed resulting in virNWFilterVarValueFree being called to clean
up @val which also cleaned up the input @addr. Thus the caller had
no way to determine on failure whether it too should clean up the
passed parameter.

Instead, let's create a copy of the input @addr, then handle that
properly in the API allowing/forcing the caller to free it's own
copy of the input parameter.
---
 src/conf/nwfilter_ipaddrmap.c     | 13 +++++++++++--
 src/nwfilter/nwfilter_dhcpsnoop.c |  3 ---
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/conf/nwfilter_ipaddrmap.c b/src/conf/nwfilter_ipaddrmap.c
index 9c8584ce27..54e6d0f0f4 100644
--- a/src/conf/nwfilter_ipaddrmap.c
+++ b/src/conf/nwfilter_ipaddrmap.c
@@ -26,7 +26,9 @@
 
 #include "internal.h"
 
+#include "viralloc.h"
 #include "virerror.h"
+#include "virstring.h"
 #include "datatypes.h"
 #include "nwfilter_params.h"
 #include "nwfilter_ipaddrmap.h"
@@ -51,28 +53,35 @@ int
 virNWFilterIPAddrMapAddIPAddr(const char *ifname, char *addr)
 {
     int ret = -1;
+    char *addrCopy;
     virNWFilterVarValuePtr val;
 
+    if (VIR_STRDUP(addrCopy, addr) < 0)
+        return -1;
+
     virMutexLock(&ipAddressMapLock);
 
     val = virHashLookup(ipAddressMap->hashTable, ifname);
     if (!val) {
-        val = virNWFilterVarValueCreateSimple(addr);
+        val = virNWFilterVarValueCreateSimple(addrCopy);
         if (!val)
             goto cleanup;
+        addrCopy = NULL;
         ret = virNWFilterHashTablePut(ipAddressMap, ifname, val);
         if (ret < 0)
             virNWFilterVarValueFree(val);
         goto cleanup;
     } else {
-        if (virNWFilterVarValueAddValue(val, addr) < 0)
+        if (virNWFilterVarValueAddValue(val, addrCopy) < 0)
             goto cleanup;
+        addrCopy = NULL;
     }
 
     ret = 0;
 
  cleanup:
     virMutexUnlock(&ipAddressMapLock);
+    VIR_FREE(addrCopy);
 
     return ret;
 }
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
index 4436e396ff..743136277d 100644
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
@@ -476,9 +476,6 @@ virNWFilterSnoopIPLeaseInstallRule(virNWFilterSnoopIPLeasePtr ipl,
     if (virNWFilterIPAddrMapAddIPAddr(req->ifname, ipaddr) < 0)
         goto exit_snooprequnlock;
 
-    /* ipaddr now belongs to the map */
-    ipaddr = NULL;
-
     if (!instantiate) {
         rc = 0;
         goto exit_snooprequnlock;
-- 
GitLab