diff options
author | Peter Wu <peter@lekensteyn.nl> | 2018-10-10 14:46:14 +0200 |
---|---|---|
committer | Anders Broman <a.broman58@gmail.com> | 2018-10-12 05:07:33 +0000 |
commit | 0e0e56d05bfc34026a9923f847b8c8d53136256f (patch) | |
tree | 5e7e74975e6ec50e1e274e27d9a44fdcc5f6572e /epan/dissectors/packet-dcerpc.c | |
parent | ec5adb0ce98c37c8119feb972a5207e0e1721d9b (diff) |
DCERPC: simplify pointer list tracking
Observe that the "current_depth" and "len_ndr_pointer_list" just track
the length of the current singly linked list in order to insert (append)
or remove [the last] element (a linked list of lists and a linked list
of pointers respectively). Replace these callers by equivalents that do
not require explicit length tracking, internally they both have to do a
O(n) lookup anyway.
There used to be a case where "current_depth" could run out-of-sync, no
longer tracking the actual list length: when the callback (tnpd->fnct or
tnpd->callback) triggers an exception. I believe this was unintentional.
No functional change intended, but this should make further changes to
the data structures easier.
Change-Id: I3cb13aba22caa87dc7baba411cf34f47792f7bb7
Ping-Bug: 14735
Fixes: v2.5.0rc0-292-g6bd87bdd5d ("dcerpc: improve greatly the speed of processing of DCERPC packets")
Reviewed-on: https://code.wireshark.org/review/30114
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Diffstat (limited to 'epan/dissectors/packet-dcerpc.c')
-rw-r--r-- | epan/dissectors/packet-dcerpc.c | 51 |
1 files changed, 20 insertions, 31 deletions
diff --git a/epan/dissectors/packet-dcerpc.c b/epan/dissectors/packet-dcerpc.c index e21b596c15..7643ba0f8c 100644 --- a/epan/dissectors/packet-dcerpc.c +++ b/epan/dissectors/packet-dcerpc.c @@ -2875,9 +2875,6 @@ dissect_ndr_wchar_vstring(tvbuff_t *tvb, int offset, packet_info *pinfo, FALSE, NULL); } -static int current_depth = 0; -static int len_ndr_pointer_list = 0; - /* ndr pointer handling */ /* Should we re-read the size of the list ? * Instead of re-calculating the size everytime, use the stored value unless this @@ -2929,7 +2926,6 @@ void init_ndr_pointer_list(dcerpc_info *di) { di->conformant_run = 0; - current_depth = 0; while (list_ndr_pointer_list) { GSList *list = (GSList *)g_slist_nth_data(list_ndr_pointer_list, 0); @@ -2943,13 +2939,12 @@ init_ndr_pointer_list(dcerpc_info *di) must_check_size = FALSE; ndr_pointer_list = create_empty_list(); - list_ndr_pointer_list = g_slist_insert(list_ndr_pointer_list, - ndr_pointer_list, 0); + list_ndr_pointer_list = g_slist_append(list_ndr_pointer_list, + ndr_pointer_list); if (ndr_pointer_hash) { g_hash_table_destroy(ndr_pointer_hash); } ndr_pointer_hash = g_hash_table_new(g_int_hash, g_int_equal); - len_ndr_pointer_list = 1; } int @@ -2958,15 +2953,17 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ int found_new_pointer; int old_offset; int next_pointer; - int original_depth; + unsigned original_depth; int len; GSList *current_ndr_pointer_list; + ndr_pointer_list = NULL; next_pointer = 0; - original_depth = current_depth; - current_ndr_pointer_list = (GSList *)g_slist_nth_data(list_ndr_pointer_list, current_depth); + /* Obtain the current list of pointers at this level. */ + current_ndr_pointer_list = (GSList *)g_slist_last(list_ndr_pointer_list)->data; + original_depth = g_slist_length(list_ndr_pointer_list); len = g_slist_length(current_ndr_pointer_list); do { @@ -2977,7 +2974,6 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ ndr_pointer_data_t *tnpd = (ndr_pointer_data_t *)g_slist_nth_data(current_ndr_pointer_list, i); if (tnpd->fnct) { - int saved_len_ndr_pointer_list = 0; GSList *saved_ndr_pointer_list = NULL; dcerpc_dissect_fnct_t *fnct; @@ -2992,10 +2988,7 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ di->conformant_run = 1; di->conformant_eaten = 0; old_offset = offset; - current_depth++; saved_ndr_pointer_list = current_ndr_pointer_list; - saved_len_ndr_pointer_list = len_ndr_pointer_list; - len_ndr_pointer_list = 1; ndr_pointer_list = create_empty_list(); offset = (*(fnct))(tvb, offset, pinfo, NULL, di, drep); @@ -3052,22 +3045,21 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ if (tnpd->callback) tnpd->callback(pinfo, tnpd->tree, tnpd->item, di, tvb, old_offset, offset, tnpd->callback_args); proto_item_set_len(tnpd->item, offset - old_offset); - if (len_ndr_pointer_list > 1) { + if (ndr_pointer_list->next) { /* We found some pointers to dissect let's create one more level */ - len = len_ndr_pointer_list; + len = g_slist_length(ndr_pointer_list); current_ndr_pointer_list = ndr_pointer_list; /* So we will arrive right away at the second element of the list * but that's not too importnt because the first one is always empty */ i = next_pointer = 0; - list_ndr_pointer_list = g_slist_insert(list_ndr_pointer_list, - ndr_pointer_list, current_depth); + /* Save the old pointer list before moving to the next. */ + list_ndr_pointer_list = g_slist_append(list_ndr_pointer_list, + ndr_pointer_list); ndr_pointer_list = create_empty_list(); continue; } else { - current_depth--; current_ndr_pointer_list = saved_ndr_pointer_list; - len_ndr_pointer_list = saved_len_ndr_pointer_list; } } if (i == (len - 1) && (must_check_size == TRUE)) { @@ -3079,12 +3071,11 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ /* We reached the end of one level, go to the level bellow if possible * reset list a level n */ - if ((i >= (len - 1)) && (current_depth > original_depth)) { + if ((i >= (len - 1)) && (g_slist_length(list_ndr_pointer_list) > original_depth)) { GSList *list; /* Remove existing list */ g_slist_free_full(current_ndr_pointer_list, g_free); - list = (GSList *)g_slist_nth_data(list_ndr_pointer_list, current_depth); - current_depth--; + list = (GSList *)g_slist_last(list_ndr_pointer_list)->data; list_ndr_pointer_list = g_slist_remove(list_ndr_pointer_list, list); /* Rewind on the lower level, in theory it's not too great because we @@ -3092,18 +3083,18 @@ dissect_deferred_pointers(packet_info *pinfo, tvbuff_t *tvb, int offset, dcerpc_ * In practice it shouldn't be that bad ! */ next_pointer = 0; - current_ndr_pointer_list = (GSList *)g_slist_nth_data(list_ndr_pointer_list, current_depth); + /* Move to the next list of pointers. */ + current_ndr_pointer_list = (GSList *)g_slist_last(list_ndr_pointer_list)->data; len = g_slist_length(current_ndr_pointer_list); - len_ndr_pointer_list = len; found_new_pointer = 1; } } while (found_new_pointer); - DISSECTOR_ASSERT(original_depth == current_depth); + DISSECTOR_ASSERT(original_depth == g_slist_length(list_ndr_pointer_list)); g_slist_free_full(ndr_pointer_list, g_free); - ndr_pointer_list = (GSList *)g_slist_nth_data(list_ndr_pointer_list, current_depth); - len_ndr_pointer_list = g_slist_length(ndr_pointer_list); + /* Restore the previous list of pointers. */ + ndr_pointer_list = (GSList *)g_slist_last(list_ndr_pointer_list)->data; return offset; } @@ -3163,10 +3154,8 @@ add_pointer_to_list(packet_info *pinfo, proto_tree *tree, proto_item *item, p_id = wmem_new(wmem_file_scope(), guint); *p_id = id; - ndr_pointer_list = g_slist_insert(ndr_pointer_list, npd, - len_ndr_pointer_list); + ndr_pointer_list = g_slist_append(ndr_pointer_list, npd); g_hash_table_insert(ndr_pointer_hash, p_id, p_id); - len_ndr_pointer_list++; must_check_size = TRUE; } |