file: fix packet list update after dfilter change during live capture

During live captures, "cf->state==FILE_READ_IN_PROGRESS" holds and as
such setting "cf->redissection_queued" from "cf_filter_packets" will
prevent the packet list from being updated (no new packets are added and
display filter changes are not applied).

Fix this by not checking "cf->state" and instead perform an explicit
check to detect the "update_progress_dlg" issue (see original commit).
As "cf->read_lock" is implied by "cf->redissecting", remove that check
as well (see "rescan_packets").

Print a warning instead of aborting in "cf_read" since I am not sure if
that condition is currently prevented by its callers.

Bug: 14918
Change-Id: Ieb7d1ae3cbeef18f17c850ae3778822ee625dc68
Fixes: v2.9.0rc0-1110-g8e07b778f6 ("file: do not perform recursive redissections to avoid crashes")
Reviewed-on: https://code.wireshark.org/review/28538
Petri-Dish: Anders Broman <a.broman58@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>

1 files changed, 24 insertions, 3 deletions

diff --git a/file.c b/file.c
index 97144cd50e..6842a0ecf6 100644
--- a/file.c
+++ b/file.c
@@ -499,6 +499,18 @@ cf_read(capture_file *cf, gboolean reloading)
   gboolean             compiled;
   volatile gboolean    is_read_aborted = FALSE;
 
+  /* The update_progress_dlg call below might end up accepting a user request to
+   * trigger redissection/rescans which can modify/destroy the dissection
+   * context ("cf->epan"). That condition should be prevented by callers, but in
+   * case it occurs let's fail gracefully.
+   */
+  if (cf->read_lock) {
+    g_warning("Failing due to recursive cf_read(\"%s\", %d) call!",
+              cf->filename, reloading);
+    return CF_READ_ERROR;
+  }
+  cf->read_lock = TRUE;
+
   /* Compile the current display filter.
    * We assume this will not fail since cf->dfilter is only set in
    * cf_filter IFF the filter was valid.
@@ -680,6 +692,10 @@ cf_read(capture_file *cf, gboolean reloading)
     packet_list_select_first_row();
   }
 
+  /* It is safe again to execute redissections. */
+  g_assert(cf->read_lock);
+  cf->read_lock = FALSE;
+
   if (is_read_aborted) {
     /*
      * Well, the user decided to exit Wireshark while reading this *offline*
@@ -1442,7 +1458,7 @@ cf_filter_packets(capture_file *cf, gchar *dftext, gboolean force)
    * If a dissection is already in progress, queue it.
    */
   if (cf->redissection_queued == RESCAN_NONE) {
-    if (cf->state == FILE_READ_IN_PROGRESS || cf->redissecting) {
+    if (cf->read_lock) {
       cf->redissection_queued = RESCAN_SCAN;
     } else if (cf->state != FILE_CLOSED) {
       if (dftext == NULL) {
@@ -1468,8 +1484,7 @@ cf_reftime_packets(capture_file *cf)
 void
 cf_redissect_packets(capture_file *cf)
 {
-  if (cf->state == FILE_READ_IN_PROGRESS || cf->redissecting ||
-      cf->redissection_queued == RESCAN_SCAN) {
+  if (cf->read_lock || cf->redissection_queued == RESCAN_SCAN) {
     /* Dissection in progress, signal redissection rather than rescanning. That
      * would destroy the current (in-progress) dissection in "cf_read" which
      * will cause issues when "cf_read" tries to add packets to the list.
@@ -1547,6 +1562,8 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb
 
   /* Rescan in progress, clear pending actions. */
   cf->redissection_queued = RESCAN_NONE;
+  g_assert(!cf->read_lock);
+  cf->read_lock = TRUE;
 
   /* Compile the current display filter.
    * We assume this will not fail since cf->dfilter is only set in
@@ -1877,6 +1894,10 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb
   /* Cleanup and release all dfilter resources */
   dfilter_free(dfcode);
 
+  /* It is safe again to execute redissections. */
+  g_assert(cf->read_lock);
+  cf->read_lock = FALSE;
+
   /* If another rescan (due to dfilter change) or redissection (due to profile
    * change) was requested, the rescan above is aborted and restarted here. */
   if (queued_rescan_type != RESCAN_NONE) {