Bug #582 fix plugin cleanups to fix memory leaks

docs/CHANGELOG

@@ -16,6 +16,7 @@
     - fix UNUSED macro declaration (#614)
     - handle malformed and unsupported packets as soft errors (#613)
     - compile failure on aarch64-linux-android (#612)
+    - tcprewrite --fixlen not working (#582)
     - fix configure --without-libdnet (#567)
     - ensure automake version is at least 1.15 (#553)
     - with multiplier option only first file can be sent and hang (#472)

src/common/list.c

@@ -80,10 +80,10 @@ parse_list(tcpr_list_t ** listdata, char *ourstr)
     /* regex test */
     if (regexec(&preg, this, 0, NULL, 0) != 0) {
         warnx("Unable to parse: %s", this);
+        regfree(&preg);
         return 0;
     }
 
-
     *listdata = new_list();
     list_ptr = *listdata;
     listcur = list_ptr;
@@ -115,6 +115,7 @@ parse_list(tcpr_list_t ** listdata, char *ourstr)
         /* regex test */
         if (regexec(&preg, this, 0, NULL, 0) != 0) {
             warnx("Unable to parse: %s", this);
+            regfree(&preg);
             return 0;
         }
 
@@ -138,6 +139,8 @@ parse_list(tcpr_list_t ** listdata, char *ourstr)
 
     }
 
+    regfree(&preg);
+
     return 1;
 }
 

src/common/services.c

@@ -91,5 +91,6 @@ parse_services(const char *file, tcpr_services_t *services)
         }
     }
 
+    regfree(&preg);
     fclose(service);
 }

src/tcpbridge.c

@@ -79,12 +79,14 @@ main(int argc, char *argv[])
     /* parse the tcpedit args */
     rcode = tcpedit_post_args(tcpedit);
     if (rcode < 0) {
+        tcpedit_close(&tcpedit);
         errx(-1, "Unable to parse args: %s", tcpedit_geterr(tcpedit));
     } else if (rcode == 1) {
         warnx("%s", tcpedit_geterr(tcpedit));
     }
 
     if (tcpedit_validate(tcpedit) < 0) {
+        tcpedit_close(&tcpedit);
         errx(-1, "Unable to edit packets given options:\n%s",
                 tcpedit_geterr(tcpedit));
     }
@@ -96,9 +98,10 @@ main(int argc, char *argv[])
     }
 #endif
 
-    if (gettimeofday(&stats.start_time, NULL) < 0)
+    if (gettimeofday(&stats.start_time, NULL) < 0) {
+        tcpedit_close(&tcpedit);
         err(-1, "gettimeofday() failed");
-
+    }
 
     /* process packets */
     do_bridge(&options, tcpedit);
@@ -110,6 +113,7 @@ main(int argc, char *argv[])
         pcap_close(options.pcap2);
     }
 
+    tcpedit_close(&tcpedit);
 #ifdef ENABLE_VERBOSE
     tcpdump_close(options.tcpdump);
 #endif

src/tcpedit/plugins/dlt_en10mb/en10mb.c

@@ -104,8 +104,17 @@ dlt_en10mb_init(tcpeditdlt_t *ctx)
         return TCPEDIT_ERROR;
     }
 
-    ctx->decoded_extra_size = sizeof(en10mb_extra_t);
-    ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    if (ctx->decoded_extra_size > 0) {
+        if (ctx->decoded_extra_size < sizeof(en10mb_extra_t)) {
+            ctx->decoded_extra_size = sizeof(en10mb_extra_t);
+            ctx->decoded_extra = safe_realloc(ctx->decoded_extra,
+                                              ctx->decoded_extra_size);
+        }
+    } else {
+        ctx->decoded_extra_size = sizeof(en10mb_extra_t);
+        ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    }
+
     plugin->config_size = sizeof(en10mb_config_t);
     plugin->config = safe_malloc(plugin->config_size);
     config = (en10mb_config_t *)plugin->config;
@@ -128,24 +137,35 @@ int
 dlt_en10mb_cleanup(tcpeditdlt_t *ctx)
 {
     tcpeditdlt_plugin_t *plugin;
-    
+
     assert(ctx);
-
-    if ((plugin = tcpedit_dlt_getplugin(ctx, dlt_value)) == NULL)
-        return TCPEDIT_OK;
+
+    plugin = ctx->plugins;
+    if (plugin == NULL) {
+        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s",
+                       dlt_name);
+        return TCPEDIT_ERROR;
+    }
+
+    while (plugin != NULL) {
+        safe_free(plugin->name);
+        plugin->name = NULL;
+        if (plugin->config) {
+            en10mb_config_t *config = (en10mb_config_t*)plugin->config;
+            safe_free(config->subs.entries);
+            safe_free(plugin->config);
+        }
+        plugin->config = NULL;
+        plugin->config_size = 0;
+        plugin = plugin->next;
+    }
 
     if (ctx->decoded_extra != NULL) {
         safe_free(ctx->decoded_extra);
         ctx->decoded_extra = NULL;
         ctx->decoded_extra_size = 0;
     }
 
-    if (plugin->config != NULL) {
-        safe_free(plugin->config);
-        plugin->config = NULL;
-        plugin->config_size = 0;
-    }
-
     return TCPEDIT_OK; /* success */
 }
 
@@ -164,14 +184,16 @@ en10mb_sub_entry_t *
 dlt_en10mb_realloc_merge(en10mb_sub_conf_t config, en10mb_sub_entry_t *new_entries, int entries_count)
 {
     int i;
-    en10mb_sub_entry_t *merged = safe_realloc(
-        config.entries, (config.count + entries_count) * sizeof(en10mb_sub_entry_t));
+
+    config.entries = safe_realloc(config.entries,
+                                  (config.count + entries_count)
+                                  * sizeof(en10mb_sub_entry_t));
 
     for (i = 0; i < entries_count; i++) {
-        merged[config.count + i] = new_entries[i];
+        config.entries[config.count + i] = new_entries[i];
     }
 
-    return merged;
+    return config.entries;
 }
 
 int

src/tcpedit/plugins/dlt_hdlc/hdlc.c

@@ -102,8 +102,16 @@ dlt_hdlc_init(tcpeditdlt_t *ctx)
     }
 
     /* allocate memory for our deocde extra data */
-    ctx->decoded_extra_size = sizeof(hdlc_extra_t);
-    ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    if (ctx->decoded_extra_size > 0) {
+        if (ctx->decoded_extra_size < sizeof(hdlc_extra_t)) {
+            ctx->decoded_extra_size = sizeof(hdlc_extra_t);
+            ctx->decoded_extra = safe_realloc(ctx->decoded_extra,
+                                              ctx->decoded_extra_size);
+        }
+    } else {
+        ctx->decoded_extra_size = sizeof(hdlc_extra_t);
+        ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    }
 
     /* allocate memory for our config data */
     plugin->config_size = sizeof(hdlc_config_t);
@@ -127,22 +135,27 @@ dlt_hdlc_cleanup(tcpeditdlt_t *ctx)
     tcpeditdlt_plugin_t *plugin;
     assert(ctx);
 
-    if ((plugin = tcpedit_dlt_getplugin(ctx, dlt_value)) == NULL) {
-        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s", dlt_name);
+    plugin = ctx->plugins;
+    if (plugin == NULL) {
+        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s",
+                       dlt_name);
         return TCPEDIT_ERROR;
     }
 
+    while (plugin != NULL) {
+        safe_free(plugin->name);
+        plugin->name = NULL;
+        safe_free(plugin->config);
+        plugin->config = NULL;
+        plugin->config_size = 0;
+        plugin = plugin->next;
+    }
+
     if (ctx->decoded_extra != NULL) {
         safe_free(ctx->decoded_extra);
         ctx->decoded_extra = NULL;
         ctx->decoded_extra_size = 0;
     }
-
-    if (plugin->config != NULL) {
-        safe_free(plugin->config);
-        plugin->config = NULL;
-        plugin->config_size = 0;
-    }
 
     return TCPEDIT_OK; /* success */
 }

src/tcpedit/plugins/dlt_ieee80211/ieee80211.c

@@ -109,8 +109,16 @@ dlt_ieee80211_init(tcpeditdlt_t *ctx)
     }
 
     /* allocate memory for our decode extra data */
-    ctx->decoded_extra_size = sizeof(ieee80211_extra_t);
-    ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    if (ctx->decoded_extra_size > 0) {
+        if (ctx->decoded_extra_size < sizeof(ieee80211_extra_t)) {
+            ctx->decoded_extra_size = sizeof(ieee80211_extra_t);
+            ctx->decoded_extra = safe_realloc(ctx->decoded_extra,
+                                              ctx->decoded_extra_size);
+        }
+    } else {
+        ctx->decoded_extra_size = sizeof(ieee80211_extra_t);
+        ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    }
 
     /* allocate memory for our config data */
     plugin->config_size = sizeof(ieee80211_config_t);
@@ -132,22 +140,27 @@ dlt_ieee80211_cleanup(tcpeditdlt_t *ctx)
     tcpeditdlt_plugin_t *plugin;
     assert(ctx);
 
-    if ((plugin = tcpedit_dlt_getplugin(ctx, dlt_value)) == NULL) {
-        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s", dlt_name);
+    plugin = ctx->plugins;
+    if (plugin == NULL) {
+        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s",
+                       dlt_name);
         return TCPEDIT_ERROR;
     }
 
+    while (plugin != NULL) {
+        safe_free(plugin->name);
+        plugin->name = NULL;
+        safe_free(plugin->config);
+        plugin->config = NULL;
+        plugin->config_size = 0;
+        plugin = plugin->next;
+    }
+
     if (ctx->decoded_extra != NULL) {
         safe_free(ctx->decoded_extra);
         ctx->decoded_extra = NULL;
         ctx->decoded_extra_size = 0;
     }
-
-    if (plugin->config != NULL) {
-        safe_free(plugin->config);
-        plugin->config = NULL;
-        plugin->config_size = 0;
-    }
 
     return TCPEDIT_OK; /* success */
 }

src/tcpedit/plugins/dlt_linuxsll/linuxsll.c

@@ -104,8 +104,16 @@ dlt_linuxsll_init(tcpeditdlt_t *ctx)
     }
 
     /* allocate memory for our decode extra data */
-    ctx->decoded_extra_size = sizeof(linuxsll_extra_t);
-    ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    if (ctx->decoded_extra_size > 0) {
+        if (ctx->decoded_extra_size < sizeof(linuxsll_extra_t)) {
+            ctx->decoded_extra_size = sizeof(linuxsll_extra_t);
+            ctx->decoded_extra = safe_realloc(ctx->decoded_extra,
+                                              ctx->decoded_extra_size);
+        }
+    } else {
+        ctx->decoded_extra_size = sizeof(linuxsll_extra_t);
+        ctx->decoded_extra = safe_malloc(ctx->decoded_extra_size);
+    }
 
     /* allocate memory for our config data */
     plugin->config_size = sizeof(linuxsll_config_t);
@@ -125,24 +133,29 @@ dlt_linuxsll_cleanup(tcpeditdlt_t *ctx)
     tcpeditdlt_plugin_t *plugin;
     assert(ctx);
 
-    if ((plugin = tcpedit_dlt_getplugin(ctx, dlt_value)) == NULL) {
-        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s", dlt_name);
+    plugin = ctx->plugins;
+    if (plugin == NULL) {
+        tcpedit_seterr(ctx->tcpedit, "Unable to cleanup unregistered plugin %s",
+                       dlt_name);
         return TCPEDIT_ERROR;
     }
 
+    while (plugin != NULL) {
+        safe_free(plugin->name);
+        plugin->name = NULL;
+        safe_free(plugin->config);
+        plugin->config = NULL;
+        plugin->config_size = 0;
+        plugin = plugin->next;
+    }
+
     /* FIXME: make this function do something if necessary */
     if (ctx->decoded_extra != NULL) {
         safe_free(ctx->decoded_extra);
         ctx->decoded_extra = NULL;
         ctx->decoded_extra_size = 0;
     }
 
-    if (plugin->config != NULL) {
-        safe_free(plugin->config);
-        plugin->config = NULL;
-        plugin->config_size = 0;
-    }
-
     return TCPEDIT_OK; /* success */
 }
 

src/tcpedit/plugins/dlt_plugins.c

@@ -452,6 +452,8 @@ tcpedit_dlt_dst(tcpeditdlt_t *ctx)
 void
 tcpedit_dlt_cleanup(tcpeditdlt_t *ctx)
 {
+    tcpeditdlt_plugin_t *plugin;
+
     assert(ctx);
 
     if (ctx->encoder != NULL)
@@ -460,12 +462,26 @@ tcpedit_dlt_cleanup(tcpeditdlt_t *ctx)
     if (ctx->decoder != NULL)
         ctx->decoder->plugin_cleanup(ctx);
 
+    if (ctx->plugins != NULL)
+        ctx->plugins->plugin_cleanup(ctx);
+
+    plugin = ctx->plugins;
+    while (plugin != NULL) {
+        tcpeditdlt_plugin_t *plugin_next = plugin->next;
+        safe_free(plugin);
+        plugin = plugin_next;
+    }
+
 #ifdef FORCE_ALIGN
     safe_free(ctx->l3buff);
 #endif
 
-    if (ctx->decoded_extra != NULL)
+    if (ctx->decoded_extra != NULL) {
+        printf("dlt_plugins.c: free ctx->decoded_extra=%p\n", ctx->decoded_extra);
         safe_free(ctx->decoded_extra);
+        ctx->decoded_extra = NULL;
+        ctx->decoded_extra_size = 0;
+    }
 
     safe_free(ctx);
 }