From b230aa1df03b5465bd0bbfdb3fe15b4fca443a21 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Thu, 19 Jan 2023 12:34:01 -0500 Subject: [PATCH] epan: Do not add dependent packets more than once Do not add a dependent frame if it's already been added to a frame's list. Do not mark a frame as a dependent of a displayed frame if we've already marked it as such in this pass. Clear the list of dependent frames if we reset the frame data, because the list of dependent frames depends on the dissection and may not be valid if redissecting (because, for example, a reassembly preference may have changed.) Move the pointer to the list of dependent frames away from the bitfields to a location that minimizes the struct size. Fixup f870c6085dc3d34c68eae36b Fix #18809 --- epan/frame_data.c | 5 +++++ epan/frame_data.h | 2 +- epan/frame_data_sequence.c | 9 +++++++-- epan/packet.c | 11 ++++++++++- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/epan/frame_data.c b/epan/frame_data.c index 23640abc72..59673d0f0d 100644 --- a/epan/frame_data.c +++ b/epan/frame_data.c @@ -298,6 +298,11 @@ frame_data_reset(frame_data *fdata) g_slist_free(fdata->pfd); fdata->pfd = NULL; } + + if (fdata->dependent_frames) { + g_slist_free(fdata->dependent_frames); + fdata->dependent_frames = NULL; + } } void diff --git a/epan/frame_data.h b/epan/frame_data.h index 55d2fa99c0..b631cac2ac 100644 --- a/epan/frame_data.h +++ b/epan/frame_data.h @@ -71,13 +71,13 @@ typedef struct _frame_data { LLP64 (64-bit Windows) platforms. Put them here, one after the other, so they don't require padding between them. */ GSList *pfd; /**< Per frame proto data */ + GSList *dependent_frames; /**< A list of frames which this one depends on */ const struct _color_filter *color_filter; /**< Per-packet matching color_filter_t object */ guint16 subnum; /**< subframe number, for protocols that require this */ /* Keep the bitfields below to 16 bits, so this plus the previous field are 32 bits. */ unsigned int passed_dfilter : 1; /**< 1 = display, 0 = no display */ unsigned int dependent_of_displayed : 1; /**< 1 if a displayed frame depends on this frame */ - GSList *dependent_frames; /**< A list of frames which this one depends on */ /* Do NOT use packet_char_enc enum here: MSVC compiler does not handle an enum in a bit field properly */ unsigned int encoding : 1; /**< Character encoding (ASCII, EBCDIC...) */ unsigned int visited : 1; /**< Has this packet been visited yet? 1=Yes,0=No*/ diff --git a/epan/frame_data_sequence.c b/epan/frame_data_sequence.c index 5ae3703d37..144d6c1fcb 100644 --- a/epan/frame_data_sequence.c +++ b/epan/frame_data_sequence.c @@ -320,8 +320,13 @@ find_and_mark_frame_depended_upon(gpointer data, gpointer user_data) if (dependent_frame && frames) { dependent_fd = frame_data_sequence_find(frames, dependent_frame); - dependent_fd->dependent_of_displayed = 1; - g_slist_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames); + /* Don't recurse for packets we've already marked. Note we assume that no + * packet depends on a future packet; we assume that in other places too. + */ + if (!(dependent_fd->dependent_of_displayed || dependent_fd->passed_dfilter)) { + dependent_fd->dependent_of_displayed = 1; + g_slist_foreach(dependent_fd->dependent_frames, find_and_mark_frame_depended_upon, frames); + } } } diff --git a/epan/packet.c b/epan/packet.c index 96a6e5bed8..204e17696d 100644 --- a/epan/packet.c +++ b/epan/packet.c @@ -449,7 +449,16 @@ mark_frame_as_depended_upon(frame_data *fd, guint32 frame_num) { /* Don't mark a frame as dependent on itself */ if (frame_num != fd->num) { - fd->dependent_frames = g_slist_prepend(fd->dependent_frames, GUINT_TO_POINTER(frame_num)); + /* ws_assert(frame_num < fd->num) - we assume in several other + * places in the code that frames don't depend on future + * frames. */ + /* XXX: Looking to see if the frame is already there is slow + * if there's a lot of dependent frames, so this should + * be a hash table or something. + */ + if (g_slist_find(fd->dependent_frames, GUINT_TO_POINTER(frame_num)) == NULL) { + fd->dependent_frames = g_slist_prepend(fd->dependent_frames, GUINT_TO_POINTER(frame_num)); + } } }