osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

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>
This commit is contained in:
Peter Wu 2018-10-10 14:46:14 +02:00 committed by Anders Broman
parent ec5adb0ce9
commit 0e0e56d05b
1 changed files with 20 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

51
epan/dissectors/packet-dcerpc.c
View File

@ -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;
}