Only store frame number and not a pointer to frame_data structure in seq_analysis_item_t

Qt GUI allows to keep windows open after closing the capture file. Let's not access to freed memory.

Bug: 11573
Change-Id: I9e70d818a4b228af319961ec512b6b9725792477
Reviewed-on: https://code.wireshark.org/review/11270
Petri-Dish: Pascal Quantin <pascal.quantin@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Gerald Combs <gerald@wireshark.org>

12 files changed, 45 insertions, 42 deletions

diff --git a/ui/gtk/graph_analysis.c b/ui/gtk/graph_analysis.c
index da81d78b01..5aa85eaa55 100644
--- a/ui/gtk/graph_analysis.c
+++ b/ui/gtk/graph_analysis.c
@@ -442,7 +442,7 @@ static void dialog_graph_draw(graph_analysis_data_t *user_data)
 		if (gai->display) {
 			if (current_item>=display_items) break;		/* the item is outside the display */
 			if (i>=first_item) {
-				user_data->dlg.items[current_item].fd = gai->fd;
+				user_data->dlg.items[current_item].frame_number = gai->frame_number;
 				user_data->dlg.items[current_item].port_src = gai->port_src;
 				user_data->dlg.items[current_item].port_dst = gai->port_dst;
 				user_data->dlg.items[current_item].frame_label = gai->frame_label;
@@ -1000,7 +1000,7 @@ static gboolean button_press_event(GtkWidget *widget _U_, GdkEventButton *event,
 	user_data->dlg.needs_redraw = TRUE;
 	dialog_graph_draw(user_data);
 
-	cf_goto_frame(&cfile, user_data->dlg.items[item].fd->num);
+	cf_goto_frame(&cfile, user_data->dlg.items[item].frame_number);
 
 	return TRUE;
 }
@@ -1035,7 +1035,7 @@ static gboolean key_press_event(GtkWidget *widget _U_, GdkEventKey *event, gpoin
 	user_data->dlg.needs_redraw = TRUE;
 	dialog_graph_draw(user_data);
 
-	cf_goto_frame(&cfile, user_data->dlg.items[user_data->dlg.selected_item-user_data->dlg.first_item].fd->num);
+	cf_goto_frame(&cfile, user_data->dlg.items[user_data->dlg.selected_item-user_data->dlg.first_item].frame_number);
 
 	return TRUE;
 }
diff --git a/ui/gtk/graph_analysis.h b/ui/gtk/graph_analysis.h
index 0fcf480d85..a22c87c46c 100644
--- a/ui/gtk/graph_analysis.h
+++ b/ui/gtk/graph_analysis.h
@@ -39,7 +39,7 @@
 #define NUM_DISPLAY_ITEMS 1000
 
 typedef struct _display_items {
-	frame_data *fd;				/**< Holds the frame number and time information */
+	guint32 frame_number;		/**< frame number */
 	guint16 port_src;
 	guint16 port_dst;
 	gchar *frame_label;			/**< the label on top of the arrow */
diff --git a/ui/gtk/lbm_uimflow_dlg.c b/ui/gtk/lbm_uimflow_dlg.c
index d1766bbc6b..8855c240c6 100644
--- a/ui/gtk/lbm_uimflow_dlg.c
+++ b/ui/gtk/lbm_uimflow_dlg.c
@@ -166,7 +166,7 @@ static int lbmc_uim_flow_graph_add_to_graph(packet_info * pinfo, const lbm_uim_s
     item = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
     copy_address(&(item->src_addr), &(pinfo->src));
     copy_address(&(item->dst_addr), &(pinfo->dst));
-    item->fd = pinfo->fd;
+    item->frame_number = pinfo->fd->num;
     item->port_src = pinfo->srcport;
     item->port_dst = pinfo->destport;
     item->protocol = g_strdup(port_type_to_str(pinfo->ptype));
@@ -263,7 +263,7 @@ static void lbmc_uim_flow_graph_on_ok_cb(GtkButton * button _U_, gpointer user_d
     while (list != NULL)
     {
         seq_analysis_item_t * seq_item = (seq_analysis_item_t *)list->data;
-        set_fd_time(cfile.epan, seq_item->fd, time_str);
+        set_fd_time(cfile.epan, frame_data_sequence_find(cfile.frames, seq_item->frame_number), time_str);
         seq_item->time_str = g_strdup(time_str);
         list = g_list_next(list);
     }
diff --git a/ui/gtk/rtp_player.c b/ui/gtk/rtp_player.c
index fd69a27ae9..08d7344581 100644
--- a/ui/gtk/rtp_player.c
+++ b/ui/gtk/rtp_player.c
@@ -399,7 +399,7 @@ mark_rtp_stream_to_play(gchar *key _U_ , rtp_stream_info_t *rsi, gpointer ptr _U
 	while (graph_list)
 	{
 		graph_item = (seq_analysis_item_t *)graph_list->data;
-		if (rsi->start_fd->num == graph_item->fd->num) {
+		if (rsi->start_fd->num == graph_item->frame_number) {
 			rsi->call_num = graph_item->conv_num;
 			/* if it is in the graph list, then check if the voip_call is selected */
 			voip_calls_list = g_queue_peek_nth_link(voip_calls->callsinfos, 0);
diff --git a/ui/gtk/voip_calls_dlg.c b/ui/gtk/voip_calls_dlg.c
index 082cd0c6dd..dd5abba795 100644
--- a/ui/gtk/voip_calls_dlg.c
+++ b/ui/gtk/voip_calls_dlg.c
@@ -232,7 +232,7 @@ voip_calls_on_filter(GtkButton *button _U_, gpointer user_data _U_)
 			while (listb) {
 				gai = (seq_analysis_item_t *)listb->data;
 				if (gai->conv_num == listinfo->call_num) {
-					g_string_append_printf(filter_string_fwd, "%sframe.number == %u", is_first?"":" or ", gai->fd->num);
+					g_string_append_printf(filter_string_fwd, "%sframe.number == %u", is_first?"":" or ", gai->frame_number);
 					is_first = FALSE;
 				}
 				listb = g_list_next(listb);
diff --git a/ui/qt/lbm_uimflow_dialog.cpp b/ui/qt/lbm_uimflow_dialog.cpp
index 01ee8477b3..c166a43fb4 100644
--- a/ui/qt/lbm_uimflow_dialog.cpp
+++ b/ui/qt/lbm_uimflow_dialog.cpp
@@ -125,7 +125,7 @@ static gboolean lbm_uimflow_add_to_graph(seq_analysis_info_t * seq_info, packet_
     item = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
     copy_address(&(item->src_addr), &(pinfo->src));
     copy_address(&(item->dst_addr), &(pinfo->dst));
-    item->fd = pinfo->fd;
+    item->frame_number = pinfo->fd->num;
     item->port_src = pinfo->srcport;
     item->port_dst = pinfo->destport;
     item->protocol = g_strdup(port_type_to_str(pinfo->ptype));
@@ -191,7 +191,7 @@ static void lbm_uimflow_get_analysis(capture_file * cfile, seq_analysis_info_t *
     while (list != NULL)
     {
         seq_analysis_item_t * seq_item = (seq_analysis_item_t *)list->data;
-        set_fd_time(cfile->epan, seq_item->fd, time_str);
+        set_fd_time(cfile->epan, frame_data_sequence_find(cfile->frames, seq_item->frame_number), time_str);
         seq_item->time_str = g_strdup(time_str);
         list = g_list_next(list);
     }
@@ -422,7 +422,7 @@ void LBMUIMFlowDialog::mouseMoved(QMouseEvent * event)
         seq_analysis_item_t * sai = m_sequence_diagram->itemForPosY(event->pos().y());
         if (sai)
         {
-            m_packet_num = sai->fd->num;
+            m_packet_num = sai->frame_number;
             hint = QString("Packet %1: %2").arg(m_packet_num).arg(sai->comment);
         }
     }
diff --git a/ui/qt/sequence_diagram.cpp b/ui/qt/sequence_diagram.cpp
index 6ba46b50f3..343db745d8 100644
--- a/ui/qt/sequence_diagram.cpp
+++ b/ui/qt/sequence_diagram.cpp
@@ -203,7 +203,7 @@ void SequenceDiagram::draw(QCPPainter *painter)
         seq_analysis_item_t *sai = it.value().value;
         QPen fg_pen(mainPen());
 
-        if (sai->fd->num == selected_packet_) {
+        if (sai->frame_number == selected_packet_) {
             // Highlighted background
             painter->save();
             QRect bg_rect(
diff --git a/ui/qt/sequence_dialog.cpp b/ui/qt/sequence_dialog.cpp
index e30fcc6564..d96036b8a9 100644
--- a/ui/qt/sequence_dialog.cpp
+++ b/ui/qt/sequence_dialog.cpp
@@ -267,7 +267,7 @@ void SequenceDialog::mouseMoved(QMouseEvent *event)
     if (event) {
         seq_analysis_item_t *sai = seq_diagram_->itemForPosY(event->pos().y());
         if (sai) {
-            packet_num_ = sai->fd->num;
+            packet_num_ = sai->frame_number;
             hint = QString("Packet %1: %2").arg(packet_num_).arg(sai->comment);
         }
     }
diff --git a/ui/qt/voip_calls_dialog.cpp b/ui/qt/voip_calls_dialog.cpp
index 486e3a604c..6897580bd9 100644
--- a/ui/qt/voip_calls_dialog.cpp
+++ b/ui/qt/voip_calls_dialog.cpp
@@ -277,7 +277,7 @@ void VoipCallsDialog::tapDraw(void *tapinfo_ptr)
             seq_analysis_item_t * sai = (seq_analysis_item_t *)graph_item->data;
             rtp_stream_info_t *rsi = (rtp_stream_info_t *)rsi_entry->data;
 
-            if (rsi->start_fd->num == sai->fd->num) {
+            if (rsi->start_fd->num == sai->frame_number) {
                 rsi->call_num = sai->conv_num;
                 // VOIP_CALLS_DEBUG("setting conv num %u for frame %u", sai->conv_num, sai->fd->num);
             }
@@ -366,7 +366,7 @@ void VoipCallsDialog::prepareFilter()
     while (cur_ga_item && cur_ga_item->data) {
         seq_analysis_item_t *ga_item = (seq_analysis_item_t*) cur_ga_item->data;
         if (selected_calls.contains(ga_item->conv_num)) {
-            filter_str += QString("%1frame.number == %2").arg(or_prepend).arg(ga_item->fd->num);
+            filter_str += QString("%1frame.number == %2").arg(or_prepend).arg(ga_item->frame_number);
             or_prepend = " or ";
         }
         cur_ga_item = g_list_next(cur_ga_item);
diff --git a/ui/tap-sequence-analysis.c b/ui/tap-sequence-analysis.c
index e4635abeb5..9ea84ee3dd 100644
--- a/ui/tap-sequence-analysis.c
+++ b/ui/tap-sequence-analysis.c
@@ -105,7 +105,7 @@ seq_analysis_frame_packet( void *ptr, packet_info *pinfo, epan_dissect_t *edt _U
 
         if (!sai) return FALSE;
 
-        sai->fd = pinfo->fd;
+        sai->frame_number = pinfo->fd->num;
 
         sai->port_src=pinfo->srcport;
         sai->port_dst=pinfo->destport;
@@ -197,7 +197,7 @@ seq_analysis_tcp_packet( void *ptr _U_, packet_info *pinfo, epan_dissect_t *edt
         seq_analysis_item_t *sai;
 
         sai = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
-        sai->fd = pinfo->fd;
+        sai->frame_number = pinfo->fd->num;
         if (sainfo->any_addr) {
             copy_address(&(sai->src_addr),&(pinfo->net_src));
             copy_address(&(sai->dst_addr),&(pinfo->net_dst));
@@ -250,8 +250,9 @@ static void sequence_analysis_item_set_timestamp(gpointer data, gpointer user_da
 {
     gchar time_str[COL_MAX_LEN];
     seq_analysis_item_t *seq_item = (seq_analysis_item_t *)data;
-    const struct epan_session *epan = (const struct epan_session *)user_data;
-    set_fd_time(epan, seq_item->fd, time_str);
+    const capture_file *cf = (const capture_file *)user_data;
+    frame_data *fd = frame_data_sequence_find(cf->frames, seq_item->frame_number);
+    set_fd_time(cf->epan, fd, time_str);
     seq_item->time_str = g_strdup(time_str);
 }
 
@@ -287,7 +288,7 @@ sequence_analysis_list_get(capture_file *cf, seq_analysis_info_t *sainfo)
 
     /* SEQ_ANALYSIS_DEBUG("%d items", g_queue_get_length(sainfo->items)); */
     /* Fill in the timestamps */
-    g_queue_foreach(sainfo->items, sequence_analysis_item_set_timestamp, cf->epan);
+    g_queue_foreach(sainfo->items, sequence_analysis_item_set_timestamp, cf);
 }
 
 static void sequence_analysis_item_free(gpointer data)
@@ -310,10 +311,10 @@ sequence_analysis_sort_compare(gconstpointer a, gconstpointer b, gpointer user_d
     const seq_analysis_item_t *entry_a = (const seq_analysis_item_t *)a;
     const seq_analysis_item_t *entry_b = (const seq_analysis_item_t *)b;
 
-    if(entry_a->fd->num < entry_b->fd->num)
+    if(entry_a->frame_number < entry_b->frame_number)
         return -1;
 
-    if(entry_a->fd->num > entry_b->fd->num)
+    if(entry_a->frame_number > entry_b->frame_number)
         return 1;
 
     return 0;
@@ -484,6 +485,7 @@ sequence_analysis_dump_to_file(const char *pathname, seq_analysis_info_t *sainfo
     guint32  i, display_items, display_nodes;
     guint32  start_position, end_position, item_width, header_length;
     seq_analysis_item_t *sai;
+    frame_data *fd;
     guint16  first_conv_num = 0;
     gboolean several_convs  = FALSE;
     gboolean first_packet   = TRUE;
@@ -631,12 +633,13 @@ sequence_analysis_dump_to_file(const char *pathname, seq_analysis_info_t *sainfo
             fprintf(of, "%s", label_string->str);
         }
 
+        fd = frame_data_sequence_find(cf->frames, sai->frame_number);
 #if 0
         /* write the time */
-        g_string_printf(label_string, "|%.3f", nstime_to_sec(&sai->fd->rel_ts));
+        g_string_printf(label_string, "|%.3f", nstime_to_sec(&fd->rel_ts));
 #endif
         /* Write the time, using the same format as in the time col */
-        set_fd_time(cf->epan, sai->fd, time_str);
+        set_fd_time(cf->epan, fd, time_str);
         g_string_printf(label_string, "|%s", time_str);
         enlarge_string(label_string, 10, ' ');
         fprintf(of, "%s", label_string->str);
diff --git a/ui/tap-sequence-analysis.h b/ui/tap-sequence-analysis.h
index daabeca205..fbebd2acbb 100644
--- a/ui/tap-sequence-analysis.h
+++ b/ui/tap-sequence-analysis.h
@@ -50,7 +50,7 @@ typedef enum seq_analysis_type_ {
 
 /** defines an entry for the graph analysis */
 typedef struct _seq_analysis_item {
-    frame_data *fd;                     /**< Holds the frame number and time information */
+    guint32 frame_number;
     address src_addr;
     guint16 port_src;
     address dst_addr;
diff --git a/ui/voip_calls.c b/ui/voip_calls.c
index fff48ef5c6..bdcbb428a8 100644
--- a/ui/voip_calls.c
+++ b/ui/voip_calls.c
@@ -310,7 +310,7 @@ add_to_graph(voip_calls_tapinfo_t *tapinfo, packet_info *pinfo, epan_dissect_t *
     }
 
     gai = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
-    gai->fd = pinfo->fd;
+    gai->frame_number = pinfo->fd->num;
     copy_address(&(gai->src_addr),src_addr);
     copy_address(&(gai->dst_addr),dst_addr);
 
@@ -330,12 +330,12 @@ add_to_graph(voip_calls_tapinfo_t *tapinfo, packet_info *pinfo, epan_dissect_t *
 
     gai->conv_num=call_num;
     gai->line_style=line_style;
-    set_fd_time(edt->session, gai->fd, time_str);
+    set_fd_time(edt->session, pinfo->fd, time_str);
     gai->time_str = g_strdup(time_str);
     gai->display=FALSE;
 
     g_queue_push_tail(tapinfo->graph_analysis->items, gai);
-    g_hash_table_insert(tapinfo->graph_analysis->ht, &gai->fd->num, gai);
+    g_hash_table_insert(tapinfo->graph_analysis->ht, &gai->frame_number, gai);
 }
 
 /****************************************************************************/
@@ -431,7 +431,7 @@ static void insert_to_graph_t38(voip_calls_tapinfo_t *tapinfo, packet_info *pinf
     gchar     time_str[COL_MAX_LEN];
 
     new_gai = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
-    new_gai->fd = packet_list_get_row_data(frame_num);
+    new_gai->frame_number = frame_num;
     copy_address(&(new_gai->src_addr),src_addr);
     copy_address(&(new_gai->dst_addr),dst_addr);
 
@@ -449,7 +449,7 @@ static void insert_to_graph_t38(voip_calls_tapinfo_t *tapinfo, packet_info *pinf
         new_gai->comment = g_strdup("");
     new_gai->conv_num=call_num;
     new_gai->line_style=line_style;
-    set_fd_time(edt->session, new_gai->fd, time_str);
+    set_fd_time(edt->session, packet_list_get_row_data(frame_num), time_str);
     new_gai->time_str = g_strdup(time_str);
     new_gai->display=FALSE;
 
@@ -460,9 +460,9 @@ static void insert_to_graph_t38(voip_calls_tapinfo_t *tapinfo, packet_info *pinf
         while (list)
         {
             gai = (seq_analysis_item_t *)list->data;
-            if (gai->fd->num > frame_num) {
+            if (gai->frame_number > frame_num) {
                 g_queue_insert_before(tapinfo->graph_analysis->items, list, new_gai);
-                g_hash_table_insert(tapinfo->graph_analysis->ht, &new_gai->fd->num, new_gai);
+                g_hash_table_insert(tapinfo->graph_analysis->ht, &new_gai->frame_number, new_gai);
                 inserted = TRUE;
                 break;
             }
@@ -472,7 +472,7 @@ static void insert_to_graph_t38(voip_calls_tapinfo_t *tapinfo, packet_info *pinf
 
         if (!inserted) {
             g_queue_push_tail(tapinfo->graph_analysis->items, new_gai);
-            g_hash_table_insert(tapinfo->graph_analysis->ht, &new_gai->fd->num, new_gai);
+            g_hash_table_insert(tapinfo->graph_analysis->ht, &new_gai->frame_number, new_gai);
         }
     }
 }
@@ -695,7 +695,7 @@ rtp_draw(void *tap_offset_ptr)
                         duration/1000,(duration%1000), rtp_listinfo->ssrc);
             } else {
                 new_gai = (seq_analysis_item_t *)g_malloc0(sizeof(seq_analysis_item_t));
-                new_gai->fd = rtp_listinfo->start_fd;
+                new_gai->frame_number = rtp_listinfo->start_fd->num;
                 copy_address(&(new_gai->src_addr),&(rtp_listinfo->src_addr));
                 copy_address(&(new_gai->dst_addr),&(rtp_listinfo->dest_addr));
                 new_gai->port_src = rtp_listinfo->src_port;
@@ -710,7 +710,7 @@ rtp_draw(void *tap_offset_ptr)
                         (rtp_listinfo->is_srtp)?"SRTP":"RTP", rtp_listinfo->packet_count,
                         duration/1000,(duration%1000), rtp_listinfo->ssrc);
                 new_gai->conv_num = conv_num;
-                set_fd_time(tapinfo->session, new_gai->fd, time_str);
+                set_fd_time(tapinfo->session, rtp_listinfo->start_fd, time_str);
                 new_gai->time_str = g_strdup(time_str);
                 new_gai->display=FALSE;
                 new_gai->line_style = 2;  /* the arrow line will be 2 pixels width */
@@ -753,14 +753,14 @@ rtp_packet_draw(void *tap_offset_ptr)
             gai = voip_calls_graph_list->data;
             conv_num = gai->conv_num;
             /* if we get the setup frame number, then get the time position to graph the RTP arrow */
-            if (rtp_listinfo->setup_frame_number == gai->fd->num) {
+            if (rtp_listinfo->setup_frame_number == gai->frame_number) {
                 /* look again from the beginning because there are cases where the Setup frame is after the RTP */
                 voip_calls_graph_list = g_list_first(tapinfo->graph_analysis->list);
                 item = 0;
                 while(voip_calls_graph_list) {
                     gai = voip_calls_graph_list->data;
                     /* if RTP was already in the Graph, just update the comment information */
-                    if (rtp_listinfo->start_fd->num == gai->fd->num) {
+                    if (rtp_listinfo->start_fd->num == gai->frame_number) {
                         duration = (guint32)(nstime_to_msec(&rtp_listinfo->stop_fd->rel_ts) - nstime_to_msec(&rtp_listinfo->start_fd->rel_ts));
                         g_free(gai->comment);
                         gai->comment = g_strdup_printf("%s Num packets:%u  Duration:%u.%03us SSRC:0x%X",
@@ -774,9 +774,9 @@ rtp_packet_draw(void *tap_offset_ptr)
                     if (!voip_calls_graph_list) item++;
 
                     /* add the RTP item to the graph if was not there*/
-                    if (rtp_listinfo->start_fd->num<gai->fd->num || !voip_calls_graph_list) {
+                    if (rtp_listinfo->start_fd->num<gai->frame_number || !voip_calls_graph_list) {
                         new_gai = g_malloc0(sizeof(seq_analysis_item_t));
-                        new_gai->fd = rtp_listinfo->start_fd;
+                        new_gai->frame_number = rtp_listinfo->start_fd->num;
                         copy_address(&(new_gai->src_addr),&(rtp_listinfo->src_addr));
                         copy_address(&(new_gai->dst_addr),&(rtp_listinfo->dest_addr));
                         new_gai->port_src = rtp_listinfo->src_port;
@@ -792,7 +792,7 @@ rtp_packet_draw(void *tap_offset_ptr)
                                                            (rtp_listinfo->is_srtp)?"SRTP":"RTP", rtp_listinfo->npackets,
                                                            duration/1000,(duration%1000), rtp_listinfo->ssrc);
                         new_gai->conv_num = conv_num;
-                        set_fd_time(cfile.epan, new_gai->fd, time_str);
+                        set_fd_time(cfile.epan, rtp_listinfo->start_fd, time_str);
                         new_gai->time_str = g_strdup(time_str);
                         new_gai->display=FALSE;
                         new_gai->line_style = 2;  /* the arrow line will be 2 pixels width */
@@ -869,7 +869,7 @@ t38_packet(void *tap_offset_ptr, packet_info *pinfo, epan_dissect_t *edt, const
         while (voip_calls_graph_list)
         {
             tmp_gai = (seq_analysis_item_t *)voip_calls_graph_list->data;
-            if (t38_info->setup_frame_number == tmp_gai->fd->num) {
+            if (t38_info->setup_frame_number == tmp_gai->frame_number) {
                 gai = tmp_gai;
                 break;
             }
@@ -2486,7 +2486,7 @@ mgcp_calls_packet(void *tap_offset_ptr, packet_info *pinfo, epan_dissect_t *edt,
         while (listGraph)
         {
             gai = (seq_analysis_item_t *)listGraph->data;
-            if (gai->fd->num == pi->req_num) {
+            if (gai->frame_number == pi->req_num) {
                 /* there is a request that match, so look the associated call with this call_num */
                 list = g_queue_peek_nth_link(tapinfo->callsinfos, 0);
                 while (list)