From 21e0a63b295a159b5876018eb03d714a4600a8eb Mon Sep 17 00:00:00 2001 From: Evan Huus Date: Wed, 29 Jan 2014 18:04:20 -0500 Subject: [PATCH] Add remove_last_data_source and fix bug 9169 The OP asked 9169 to be reopened because the capture was spewing ~40GB of output when dissected with tshark. Investigation showed this was because the HTTP dissector was requesting ONE_MORE_PACKET reassembly a lot, and TCP was adding each step as a data-source which was being printed by tshark's hex dump. This was leading to O(n^2) of output. To fix, introduce function remove_last_data_source which removes the most recent data source from the list. If the subdissector in TCP reassembly asks for ONE_MORE_PACKET, assume it hasn't added any tree items (since it shouldn't have) and remove the data source since it is unnecessary. This may break dissectors which add tree items and *then* return ONE_MORE_PACKET, since they will have their data source removed out from under them. I believe those cases should be fixed to not add tree items until they're sure they have enough data. Change-Id: Iff07f959b8b8bd1acda9bff03f7c8684901ba8aa Reviewed-on: https://code.wireshark.org/review/38 Reviewed-by: Evan Huus Tested-by: Evan Huus --- epan/dissectors/packet-tcp.c | 1 + epan/packet.c | 15 +++++++++++++++ epan/packet.h | 2 ++ 3 files changed, 18 insertions(+) diff --git a/epan/dissectors/packet-tcp.c b/epan/dissectors/packet-tcp.c index 7932eb34fd..8db6b49348 100644 --- a/epan/dissectors/packet-tcp.c +++ b/epan/dissectors/packet-tcp.c @@ -1889,6 +1889,7 @@ again: * being a new higher-level PDU that also * needs desegmentation). */ + remove_last_data_source(pinfo); fragment_set_partial_reassembly(&tcp_reassembly_table, pinfo, msp->first_frame, NULL); diff --git a/epan/packet.c b/epan/packet.c index b3a5cc13da..e89ab312df 100644 --- a/epan/packet.c +++ b/epan/packet.c @@ -324,9 +324,24 @@ add_new_data_source(packet_info *pinfo, tvbuff_t *tvb, const char *name) src = g_slice_new(struct data_source); src->tvb = tvb; src->name = g_strdup(name); + /* This could end up slow, but we should never have that many data + * sources so it probably doesn't matter */ pinfo->data_src = g_slist_append(pinfo->data_src, src); } +void +remove_last_data_source(packet_info *pinfo) +{ + struct data_source *src; + GSList *last; + + last = g_slist_last(pinfo->data_src); + src = (struct data_source *)last->data; + pinfo->data_src = g_slist_delete_link(pinfo->data_src, last); + g_free(src->name); + g_slice_free(struct data_source, src); +} + const char* get_data_source_name(const struct data_source *src) { diff --git a/epan/packet.h b/epan/packet.h index af178b81e1..47d0acc0f0 100644 --- a/epan/packet.h +++ b/epan/packet.h @@ -512,6 +512,8 @@ final_registration_all_protocols(void); */ WS_DLL_PUBLIC void add_new_data_source(packet_info *pinfo, tvbuff_t *tvb, const char *name); +/* Removes the last-added data source, if it turns out it wasn't needed */ +WS_DLL_PUBLIC void remove_last_data_source(packet_info *pinfo); /*