Fix memory leaks of dumper SHB and IDB infos
authorHadriel Kaplan <hadrielk@yahoo.com>
Thu, 20 Aug 2015 18:38:35 +0000 (14:38 -0400)
committerAnders Broman <a.broman58@gmail.com>
Fri, 21 Aug 2015 04:55:20 +0000 (04:55 +0000)
Change-Id: I6b81d3e853d503c6a81f9793957b48ab34c6808c
Reviewed-on: https://code.wireshark.org/review/10156
Petri-Dish: Hadriel Kaplan <hadrielk@yahoo.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
debian/libwiretap0.symbols
ui/gtk/file_import_dlg.c
ui/tap_export_pdu.c
wiretap/file_access.c
wiretap/merge.c
wiretap/nettrace_3gpp_32_423.c
wiretap/wtap.c
wiretap/wtap.h

index efcafcd80f4aed7b5abb0676bf5083e2a1f68bfc..81119c7c675fce9a2c1c60e42c2c0174a90823ad 100644 (file)
@@ -57,14 +57,15 @@ libwiretap.so.0 libwiretap0 #MINVER#
  wtap_file_get_shb@Base 1.99.9
  wtap_file_get_shb_comment@Base 1.99.9
  wtap_file_get_shb_for_new_file@Base 1.99.9
- wtap_free_nrb@Base 1.99.9
- wtap_free_shb@Base 1.99.9
  wtap_file_size@Base 1.9.1
  wtap_file_tsprec@Base 1.99.0
  wtap_file_type_subtype@Base 1.12.0~rc1
  wtap_file_type_subtype_short_string@Base 1.12.0~rc1
  wtap_file_type_subtype_string@Base 1.12.0~rc1
  wtap_free_extensions_list@Base 1.9.1
+ wtap_free_idb_info@Base 1.99.9
+ wtap_free_nrb@Base 1.99.9
+ wtap_free_shb@Base 1.99.9
  wtap_fstat@Base 1.9.1
  wtap_get_all_file_extensions_list@Base 1.12.0~rc1
  wtap_get_bytes_dumped@Base 1.9.1
index 06e3a5790b7d7fecff4c218c00e941fc279e504b..4700fa5b0ab1333783f40c6150306ae9e2b548be 100644 (file)
@@ -461,7 +461,6 @@ file_import_open(text_import_info_t *info)
     wtapng_iface_descriptions_t *idb_inf;
     wtapng_if_descr_t            int_data;
     GString                     *os_info_str;
-    char                        *appname;
 
     /* Choose a random name for the temporary import buffer */
     import_file_fd = create_tempfile(&tmpname, "import");
@@ -471,8 +470,6 @@ file_import_open(text_import_info_t *info)
     os_info_str = g_string_new("");
     get_os_version_info(os_info_str);
 
-    appname = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
-
     shb_hdr = g_new(wtapng_section_t,1);
     shb_hdr->section_length = -1;
     /* options */
@@ -491,7 +488,7 @@ file_import_open(text_import_info_t *info)
      * UTF-8 string containing the name of the application used to create
      * this section.
      */
-    shb_hdr->shb_user_appl  = appname;
+    shb_hdr->shb_user_appl  = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
 
 
     /* Create fake IDB info */
@@ -569,8 +566,8 @@ end:
     g_free(info->date_timestamp_format);
     g_free(info);
     g_free(capfile_name);
-    g_free(shb_hdr);
-    g_free(appname);
+    wtap_free_shb(shb_hdr);
+    wtap_free_idb_info(idb_inf);
     window_destroy(file_import_dlg_w);
 }
 
index f62373289a7acc62d782c52f80bb01db7cb5061a..f1dcacc63cf0c5b7a8950272dd8ec2ee6b8b4452 100644 (file)
@@ -107,7 +107,6 @@ exp_pdu_file_open(exp_pdu_t *exp_pdu_tap_data)
     wtapng_iface_descriptions_t *idb_inf;
     wtapng_if_descr_t            int_data;
     GString                     *os_info_str;
-    char                        *appname;
 
     /* Choose a random name for the temporary import buffer */
     import_file_fd = create_tempfile(&tmpname, "Wireshark_PDU_");
@@ -117,8 +116,6 @@ exp_pdu_file_open(exp_pdu_t *exp_pdu_tap_data)
     os_info_str = g_string_new("");
     get_os_version_info(os_info_str);
 
-    appname = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
-
     shb_hdr = g_new0(wtapng_section_t,1);
     shb_hdr->section_length = -1;
     /* options */
@@ -137,7 +134,7 @@ exp_pdu_file_open(exp_pdu_t *exp_pdu_tap_data)
      * UTF-8 string containing the name of the application used to create
      * this section.
      */
-    shb_hdr->shb_user_appl  = appname;
+    shb_hdr->shb_user_appl  = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
 
     /* Create fake IDB info */
     idb_inf = g_new(wtapng_iface_descriptions_t,1);
@@ -206,8 +203,8 @@ exp_pdu_file_open(exp_pdu_t *exp_pdu_tap_data)
 
 end:
     g_free(capfile_name);
-    g_free(shb_hdr);
-    g_free(appname);
+    wtap_free_shb(shb_hdr);
+    wtap_free_idb_info(idb_inf);
 }
 
 gboolean
index 69604e23713ff0c18de8ae90c64cc4402fd6f4eb..0ae80a99c984d8e7ac7e4c03eb3e30f098bce46f 100644 (file)
@@ -2169,10 +2169,12 @@ wtap_dump_init_dumper(int file_type_subtype, int encap, int snaplen, gboolean co
        if ((idb_inf != NULL) && (idb_inf->interface_data->len > 0)) {
                guint itf_count;
 
+               /* XXX: what free's this stuff? */
                wdh->interface_data = g_array_new(FALSE, FALSE, sizeof(wtapng_if_descr_t));
                for (itf_count = 0; itf_count < idb_inf->interface_data->len; itf_count++) {
                        file_int_data = &g_array_index(idb_inf->interface_data, wtapng_if_descr_t, itf_count);
                        if ((encap != WTAP_ENCAP_PER_PACKET) && (encap != file_int_data->wtap_encap)) {
+                               /* XXX: this does a shallow copy, not a true clone; e.g., comments are not duped */
                                memcpy(&descr, file_int_data, sizeof(wtapng_if_descr_t));
                                descr.wtap_encap = encap;
                                descr.link_type = wtap_wtap_encap_to_pcap_encap(encap);
index 9dbaf9231c0c023cf969178c9af4f8b981e135ed..46eae0bce22b376d336a3a013b7ed2f572e53101 100644 (file)
@@ -459,8 +459,8 @@ is_duplicate_idb(const wtapng_if_descr_t *idb1, const wtapng_if_descr_t *idb2)
 static gboolean
 all_idbs_are_duplicates(const merge_in_file_t *in_files, const guint in_file_count)
 {
-    const wtapng_iface_descriptions_t *first_idb_list = NULL;
-    const wtapng_iface_descriptions_t *other_idb_list = NULL;
+    wtapng_iface_descriptions_t *first_idb_list = NULL;
+    wtapng_iface_descriptions_t *other_idb_list = NULL;
     guint first_idb_list_size, other_idb_list_size;
     const wtapng_if_descr_t *first_file_idb, *other_file_idb;
     guint i, j;
@@ -482,6 +482,8 @@ all_idbs_are_duplicates(const merge_in_file_t *in_files, const guint in_file_cou
         if (other_idb_list_size != first_idb_list_size) {
             merge_debug2("merge::all_idbs_are_duplicates: sizes of IDB lists don't match: first=%u, other=%u",
                          first_idb_list_size, other_idb_list_size);
+            g_free(other_idb_list);
+            g_free(first_idb_list);
             return FALSE;
         }
 
@@ -491,13 +493,18 @@ all_idbs_are_duplicates(const merge_in_file_t *in_files, const guint in_file_cou
 
             if (!is_duplicate_idb(first_file_idb, other_file_idb)) {
                 merge_debug1("merge::all_idbs_are_duplicates: IDBs at index %d do not match, returning FALSE", j);
+                g_free(other_idb_list);
+                g_free(first_idb_list);
                 return FALSE;
             }
         }
+        g_free(other_idb_list);
     }
 
     merge_debug0("merge::all_idbs_are_duplicates: returning TRUE");
 
+    g_free(first_idb_list);
+
     return TRUE;
 }
 
@@ -847,6 +854,8 @@ merge_files(int out_fd, const gchar* out_filename, const int file_type,
     struct wtap_pkthdr *phdr, snap_phdr;
     int                 count = 0;
     gboolean            stop_flag = FALSE;
+    wtapng_section_t            *shb_hdr = NULL;
+    wtapng_iface_descriptions_t *idb_inf = NULL;
 
     g_assert(out_fd > 0);
     g_assert(in_file_count > 0);
@@ -890,9 +899,6 @@ merge_files(int out_fd, const gchar* out_filename, const int file_type,
 
     /* prepare the outfile */
     if (file_type == WTAP_FILE_TYPE_SUBTYPE_PCAPNG) {
-        wtapng_section_t            *shb_hdr = NULL;
-        wtapng_iface_descriptions_t *idb_inf = NULL;
-
         shb_hdr = create_shb_header(in_files, in_file_count, app_name);
         merge_debug0("merge_files: SHB created");
 
@@ -910,6 +916,8 @@ merge_files(int out_fd, const gchar* out_filename, const int file_type,
     if (pdh == NULL) {
         merge_close_in_files(in_file_count, in_files);
         g_free(in_files);
+        wtap_free_shb(shb_hdr);
+        wtap_free_idb_info(idb_inf);
         return MERGE_ERR_CANT_OPEN_OUTFILE;
     }
 
@@ -1030,6 +1038,8 @@ merge_files(int out_fd, const gchar* out_filename, const int file_type,
     }
 
     g_free(in_files);
+    wtap_free_shb(shb_hdr);
+    wtap_free_idb_info(idb_inf);
 
     return status;
 }
index b775455cb55ce2bfa6abbe737a85c1b7bb4d8198..ae1e82d07298acaf8d9c27241251434c6b526708 100644 (file)
@@ -274,18 +274,18 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
        int import_file_fd;
        wtap_dumper* wdh_exp_pdu;
        int   exp_pdu_file_err;
+       wtap_open_return_val result = WTAP_OPEN_MINE;
 
        /* pcapng defs */
-       wtapng_section_t            *shb_hdr;
-       wtapng_iface_descriptions_t *idb_inf;
+       wtapng_section_t            *shb_hdr = NULL;
+       wtapng_iface_descriptions_t *idb_inf = NULL;
        wtapng_if_descr_t            int_data;
        GString                     *os_info_str;
-       char                        *appname;
        gint64 file_size;
        int packet_size;
-       guint8 *packet_buf;
+       guint8 *packet_buf = NULL;
        int wrt_err;
-       gchar *wrt_err_info;
+       gchar *wrt_err_info = NULL;
        struct wtap_pkthdr phdr;
 
        gboolean do_random = FALSE;
@@ -298,8 +298,6 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
        os_info_str = g_string_new("");
        get_os_version_info(os_info_str);
 
-       appname = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
-
        shb_hdr = g_new(wtapng_section_t, 1);
        shb_hdr->section_length = -1;
        /* options */
@@ -318,7 +316,7 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
        * UTF-8 string containing the name of the application used to create
        * this section.
        */
-       shb_hdr->shb_user_appl = appname;
+       shb_hdr->shb_user_appl = g_strdup_printf("Wireshark %s", get_ws_vcs_version_info());
 
        /* Create fake IDB info */
        idb_inf = g_new(wtapng_iface_descriptions_t, 1);
@@ -347,16 +345,16 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
        wdh_exp_pdu = wtap_dump_fdopen_ng(import_file_fd, WTAP_FILE_TYPE_SUBTYPE_PCAPNG, WTAP_ENCAP_WIRESHARK_UPPER_PDU,
                                          WTAP_MAX_PACKET_SIZE, FALSE, shb_hdr, idb_inf, NULL, &exp_pdu_file_err);
        if (wdh_exp_pdu == NULL) {
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
 
-       g_free(shb_hdr);
-       g_free(appname);
-
        /* OK we've opend a new pcap-ng file and written the headers, time to do the packets, strt by finding the file size */
 
-       if ((file_size = wtap_file_size(wth, err)) == -1)
-               return WTAP_OPEN_ERROR;
+       if ((file_size = wtap_file_size(wth, err)) == -1) {
+               result = WTAP_OPEN_ERROR;
+               goto end;
+       }
 
        if (file_size > MAX_FILE_SIZE) {
                /*
@@ -366,7 +364,8 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
                *err = WTAP_ERR_BAD_FILE;
                *err_info = g_strdup_printf("mime_file: File has %" G_GINT64_MODIFIER "d-byte packet, bigger than maximum of %u",
                        file_size, MAX_FILE_SIZE);
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
        packet_size = (int)file_size;
        /* Allocate the packet buffer
@@ -393,7 +392,8 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
 
 
        if (!wtap_read_bytes(wth->fh, packet_buf + 12, packet_size, &wrt_err, &wrt_err_info)){
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
 
        /* Create the packet header */
@@ -413,13 +413,14 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
 
                case WTAP_ERR_UNWRITABLE_REC_DATA:
                        g_free(wrt_err_info);
+                       wrt_err_info = NULL;
                        break;
 
                default:
                        break;
                }
-               g_free(packet_buf);
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
 
        /* Advance *packet_buf to point at the raw file data */
@@ -446,19 +447,18 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
                /* Add the raw msg*/
                temp_val = write_packet_data(wdh_exp_pdu, &phdr, &wrt_err, &wrt_err_info, curr_pos);
                if (temp_val != WTAP_OPEN_MINE){
-                       g_free(packet_buf);
-                       return temp_val;
+                       result = temp_val;
+                       goto end;
                }
                curr_pos = next_pos;
        }
 
        /* Close the written file*/
        if (!wtap_dump_close(wdh_exp_pdu, err)){
-               g_free(packet_buf);
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
 
-       g_free(packet_buf);
        /* Now open the file for reading */
 
        /* Find out if random read was requested */
@@ -469,10 +469,17 @@ create_temp_pcapng_file(wtap *wth, int *err, gchar **err_info, nettrace_3gpp_32_
                wtap_open_offline(file_info->tmpname, WTAP_TYPE_AUTO, err, err_info, do_random);
 
        if (!file_info->wth_tmp_file){
-               return WTAP_OPEN_ERROR;
+               result = WTAP_OPEN_ERROR;
+               goto end;
        }
 
-       return WTAP_OPEN_MINE;
+end:
+       g_free(wrt_err_info);
+       g_free(packet_buf);
+       wtap_free_shb(shb_hdr);
+       wtap_free_idb_info(idb_inf);
+
+       return result;
 }
 
 wtap_open_return_val
index 2837b698520d156545e5ee6bd887ee639417c40b..f984bfce4d497278603510fdf13f5fad2ff2d093 100644 (file)
@@ -263,6 +263,54 @@ wtap_file_get_idb_info(wtap *wth)
        return idb_info;
 }
 
+static void
+wtap_free_isb_members(wtapng_if_stats_t *isb)
+{
+       if (isb) {
+               g_free(isb->opt_comment);
+       }
+}
+
+static void
+wtap_free_idb_members(wtapng_if_descr_t* idb)
+{
+       if (idb) {
+               g_free(idb->opt_comment);
+               g_free(idb->if_os);
+               g_free(idb->if_name);
+               g_free(idb->if_description);
+               g_free(idb->if_filter_str);
+               g_free(idb->if_filter_bpf_bytes);
+               if (idb->interface_statistics) {
+                       wtapng_if_stats_t *isb;
+                       guint i;
+                       for (i = 0; i < idb->interface_statistics->len; i++) {
+                               isb = &g_array_index(idb->interface_statistics, wtapng_if_stats_t, i);
+                               wtap_free_isb_members(isb);
+                       }
+                       g_array_free(idb->interface_statistics, TRUE);
+               }
+       }
+}
+
+void
+wtap_free_idb_info(wtapng_iface_descriptions_t *idb_info)
+{
+       if (idb_info == NULL)
+           return;
+
+       if (idb_info->interface_data) {
+               guint i;
+               for (i = 0; i < idb_info->interface_data->len; i++) {
+                       wtapng_if_descr_t* idb = &g_array_index(idb_info->interface_data, wtapng_if_descr_t, i);
+                       wtap_free_idb_members(idb);
+               }
+               g_array_free(idb_info->interface_data, TRUE);
+       }
+
+       g_free(idb_info);
+}
+
 gchar *
 wtap_get_debug_if_descr(const wtapng_if_descr_t *if_descr,
                         const int indent,
index 4c53b8706c75c8014d76ab90c84695ec0f5b9277..8de0c308985a5f5216a6060bc5b2d846dcc6a2cf 100644 (file)
@@ -1735,6 +1735,21 @@ void wtap_write_shb_comment(wtap *wth, gchar *comment);
 WS_DLL_PUBLIC
 wtapng_iface_descriptions_t *wtap_file_get_idb_info(wtap *wth);
 
+/**
+ * @brief Free's a interface description block and all of its members.
+ *
+ * @details This free's all of the interface descriptions inside the passed-in
+ *     struct, including their members (e.g., comments); and then free's the
+ *     passed-in struct as well.
+ *
+ * @warning Do not use this for the struct returned by
+ *     wtap_file_get_idb_info(), as that one did not create the internal
+ *     interface descriptions; for that case you can simply g_free() the new
+ *     struct.
+ */
+WS_DLL_PUBLIC
+void wtap_free_idb_info(wtapng_iface_descriptions_t *idb_info);
+
 /**
  * @brief Gets a debug string of an interface description.
  * @details Returns a newly allocated string of debug information about
@@ -1847,6 +1862,10 @@ wtap_dumper* wtap_dump_open(const char *filename, int file_type_subtype, int enc
 /**
  * @brief Opens a new capture file for writing.
  *
+ * @note The shb_hdr, idb_inf, and nrb_hdr arguments will be used until
+ *     wtap_dump_close() is called, but will not be free'd by the dumper. If
+ *     you created them, you must free them yourself after wtap_dump_close().
+ *
  * @param filename The new file's name.
  * @param file_type_subtype The WTAP_FILE_TYPE_SUBTYPE_XXX file type.
  * @param encap The WTAP_ENCAP_XXX encapsulation type (WTAP_ENCAP_PER_PACKET for multi)
@@ -1870,6 +1889,10 @@ wtap_dumper* wtap_dump_fdopen(int fd, int file_type_subtype, int encap, int snap
 /**
  * @brief Creates a dumper for an existing file descriptor.
  *
+ * @note The shb_hdr, idb_inf, and nrb_hdr arguments will be used until
+ *     wtap_dump_close() is called, but will not be free'd by the dumper. If
+ *     you created them, you must free them yourself after wtap_dump_close().
+ *
  * @param file_type_subtype The WTAP_FILE_TYPE_SUBTYPE_XXX file type.
  * @param encap The WTAP_ENCAP_XXX encapsulation type (WTAP_ENCAP_PER_PACKET for multi)
  * @param snaplen The maximum packet capture length.