]> xenbits.xensource.com Git - libvirt.git/commitdiff
nwfilter: Don't have virNWFilterIPAddrMapAddIPAddr consume input
authorJohn Ferlan <jferlan@redhat.com>
Fri, 29 Sep 2017 19:55:29 +0000 (15:55 -0400)
committerJohn Ferlan <jferlan@redhat.com>
Mon, 2 Oct 2017 10:14:32 +0000 (06:14 -0400)
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
src/nwfilter/nwfilter_dhcpsnoop.c

index 9c8584ce27c953d535eb005682014dcb5f7a5474..54e6d0f0f4a9ac86acd9a73eedd106afca292245 100644 (file)
@@ -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;
 }
index 4436e396ff972c51260b342aad5a1444666e48ef..743136277dc5c291ebc81334f52c651a671d4400 100644 (file)
@@ -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;