[libvirt] [PATCH 06/14] nwfilter: fix leaking of filter parameters upon error

Daniel P. Berrangé posted 14 patches 6 years, 7 months ago
There is a newer version of this series
[libvirt] [PATCH 06/14] nwfilter: fix leaking of filter parameters upon error
Posted by Daniel P. Berrangé 6 years, 7 months ago
The filter parameters were not correctly free'd when an error hits while
adding to the hash table.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/nwfilter/nwfilter_gentech_driver.c | 22 ++++++----------------
 src/nwfilter/nwfilter_gentech_driver.h |  2 +-
 2 files changed, 7 insertions(+), 17 deletions(-)

diff --git a/src/nwfilter/nwfilter_gentech_driver.c b/src/nwfilter/nwfilter_gentech_driver.c
index 91794dd3ad..af4411d4db 100644
--- a/src/nwfilter/nwfilter_gentech_driver.c
+++ b/src/nwfilter/nwfilter_gentech_driver.c
@@ -143,19 +143,20 @@ virNWFilterRuleInstFree(virNWFilterRuleInstPtr inst)
  */
 static int
 virNWFilterVarHashmapAddStdValues(virHashTablePtr table,
-                                  char *macaddr,
+                                  const char *macaddr,
                                   const virNWFilterVarValue *ipaddr)
 {
     virNWFilterVarValue *val;
 
     if (macaddr) {
-        val = virNWFilterVarValueCreateSimple(macaddr);
+        val = virNWFilterVarValueCreateSimpleCopyValue(macaddr);
         if (!val)
             return -1;
 
         if (virHashAddEntry(table,
                             NWFILTER_STD_VAR_MAC,
                             val) < 0) {
+            virNWFilterVarValueFree(val);
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            "%s", _("Could not add variable 'MAC' to hashmap"));
             return -1;
@@ -170,6 +171,7 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table,
         if (virHashAddEntry(table,
                             NWFILTER_STD_VAR_IP,
                             val) < 0) {
+            virNWFilterVarValueFree(val);
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            "%s", _("Could not add variable 'IP' to hashmap"));
             return -1;
@@ -192,7 +194,7 @@ virNWFilterVarHashmapAddStdValues(virHashTablePtr table,
  * Returns pointer to hashmap, NULL if an error occurred.
  */
 virHashTablePtr
-virNWFilterCreateVarHashmap(char *macaddr,
+virNWFilterCreateVarHashmap(const char *macaddr,
                             const virNWFilterVarValue *ipaddr)
 {
     virHashTablePtr table = virNWFilterHashTableCreate(0);
@@ -767,9 +769,7 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
     virNWFilterDefPtr filter;
     virNWFilterDefPtr newFilter;
     char vmmacaddr[VIR_MAC_STRING_BUFLEN] = {0};
-    char *str_macaddr = NULL;
     virNWFilterVarValuePtr ipaddr;
-    char *str_ipaddr = NULL;
 
     techdriver = virNWFilterTechDriverForName(drvname);
 
@@ -788,22 +788,15 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
         return -1;
 
     virMacAddrFormat(macaddr, vmmacaddr);
-    if (VIR_STRDUP(str_macaddr, vmmacaddr) < 0) {
-        rc = -1;
-        goto err_exit;
-    }
 
     ipaddr = virNWFilterIPAddrMapGetIPAddr(ifname);
 
-    vars1 = virNWFilterCreateVarHashmap(str_macaddr, ipaddr);
+    vars1 = virNWFilterCreateVarHashmap(vmmacaddr, ipaddr);
     if (!vars1) {
         rc = -1;
         goto err_exit;
     }
 
-    str_macaddr = NULL;
-    str_ipaddr = NULL;
-
     vars = virNWFilterCreateVarsFrom(vars1,
                                      filterparams);
     if (!vars) {
@@ -840,9 +833,6 @@ virNWFilterInstantiateFilterUpdate(virNWFilterDriverStatePtr driver,
  err_exit:
     virNWFilterObjUnlock(obj);
 
-    VIR_FREE(str_ipaddr);
-    VIR_FREE(str_macaddr);
-
     return rc;
 }
 
diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
index 67092157b8..86cc677e79 100644
--- a/src/nwfilter/nwfilter_gentech_driver.h
+++ b/src/nwfilter/nwfilter_gentech_driver.h
@@ -57,7 +57,7 @@ int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
 
 int virNWFilterTeardownFilter(const virDomainNetDef *net);
 
-virHashTablePtr virNWFilterCreateVarHashmap(char *macaddr,
+virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr,
                                        const virNWFilterVarValue *value);
 
 int virNWFilterDomainFWUpdateCB(virDomainObjPtr vm,
-- 
2.14.3

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 06/14] nwfilter: fix leaking of filter parameters upon error
Posted by Jiri Denemark 6 years, 7 months ago
On Fri, Apr 27, 2018 at 16:25:05 +0100, Daniel P. Berrangé wrote:
> The filter parameters were not correctly free'd when an error hits while
> adding to the hash table.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  src/nwfilter/nwfilter_gentech_driver.c | 22 ++++++----------------
>  src/nwfilter/nwfilter_gentech_driver.h |  2 +-
>  2 files changed, 7 insertions(+), 17 deletions(-)
...
> diff --git a/src/nwfilter/nwfilter_gentech_driver.h b/src/nwfilter/nwfilter_gentech_driver.h
> index 67092157b8..86cc677e79 100644
> --- a/src/nwfilter/nwfilter_gentech_driver.h
> +++ b/src/nwfilter/nwfilter_gentech_driver.h
> @@ -57,7 +57,7 @@ int virNWFilterInstantiateFilterLate(virNWFilterDriverStatePtr driver,
>  
>  int virNWFilterTeardownFilter(const virDomainNetDef *net);
>  
> -virHashTablePtr virNWFilterCreateVarHashmap(char *macaddr,
> +virHashTablePtr virNWFilterCreateVarHashmap(const char *macaddr,
>                                         const virNWFilterVarValue *value);

The alignment needs some adjustment. However, the function has no users
outside nwfilter_gentech_driver.c so a separate patch could make it
static.

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list