diff options
author | Peter Wu <peter@lekensteyn.nl> | 2018-04-24 22:34:26 +0200 |
---|---|---|
committer | Anders Broman <a.broman58@gmail.com> | 2018-04-25 06:57:00 +0000 |
commit | 614495138077a02a1fa09f6c59a0e6f1d783a1e4 (patch) | |
tree | 83a4e289734d639109cec2e4ef7d1b49a4545e23 /epan/dfilter | |
parent | 0de109ef57ebd6ddf25c87c599c9d8b522967a3b (diff) |
dfilter: fix memleaks with functions and slice operator
Running tools/dfilter-test.py with LSan enabled resulted in 38 test
failures due to memory leaks from "fvalue_new". Problematic dfilters:
- Return values from functions, e.g. `len(data.data) > 8` (instruction
CALL_FUNCTION invoking functions from epan/dfilter/dfunctions.c)
- Slice operator: `data.data[1:2] == aa:bb` (function mk_range)
These values end up in "registers", but as some values (from READ_TREE)
reference the proto tree, a new tracking flag ("owns_memory") is added.
Add missing tests for some functions and try to improve documentation.
Change-Id: I28e8cf872675d0a81ea7aa5fac7398257de3f47b
Reviewed-on: https://code.wireshark.org/review/27132
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Anders Broman <a.broman58@gmail.com>
Diffstat (limited to 'epan/dfilter')
-rw-r--r-- | epan/dfilter/dfilter-int.h | 1 | ||||
-rw-r--r-- | epan/dfilter/dfilter.c | 11 | ||||
-rw-r--r-- | epan/dfilter/dfvm.c | 23 |
3 files changed, 28 insertions, 7 deletions
diff --git a/epan/dfilter/dfilter-int.h b/epan/dfilter/dfilter-int.h index 785f5a3315..9d53353954 100644 --- a/epan/dfilter/dfilter-int.h +++ b/epan/dfilter/dfilter-int.h @@ -23,6 +23,7 @@ struct epan_dfilter { guint max_registers; GList **registers; gboolean *attempted_load; + gboolean *owns_memory; int *interesting_fields; int num_interesting_fields; GPtrArray *deprecated; diff --git a/epan/dfilter/dfilter.c b/epan/dfilter/dfilter.c index a975f84e84..c323ca5266 100644 --- a/epan/dfilter/dfilter.c +++ b/epan/dfilter/dfilter.c @@ -133,11 +133,10 @@ dfilter_free(dfilter_t *df) g_free(df->interesting_fields); - /* clear registers */ - for (i = 0; i < df->max_registers; i++) { - if (df->registers[i]) { - g_list_free(df->registers[i]); - } + /* Clear registers with constant values (as set by dfvm_init_const). + * Other registers were cleared on RETURN by free_register_overhead. */ + for (i = df->num_registers; i < df->max_registers; i++) { + g_list_free(df->registers[i]); } if (df->deprecated) { @@ -150,6 +149,7 @@ dfilter_free(dfilter_t *df) g_free(df->registers); g_free(df->attempted_load); + g_free(df->owns_memory); g_free(df); } @@ -350,6 +350,7 @@ dfilter_compile(const gchar *text, dfilter_t **dfp, gchar **err_msg) dfilter->max_registers = dfw->next_register; dfilter->registers = g_new0(GList*, dfilter->max_registers); dfilter->attempted_load = g_new0(gboolean, dfilter->max_registers); + dfilter->owns_memory = g_new0(gboolean, dfilter->max_registers); /* Initialize constants */ dfvm_init_const(dfilter); diff --git a/epan/dfilter/dfvm.c b/epan/dfilter/dfvm.c index f93fd50edf..322ca05812 100644 --- a/epan/dfilter/dfvm.c +++ b/epan/dfilter/dfvm.c @@ -331,14 +331,19 @@ read_tree(dfilter_t *df, proto_tree *tree, header_field_info *hfinfo, int reg) } df->registers[reg] = fvalues; + // These values are referenced only, do not try to free it later. + df->owns_memory[reg] = FALSE; return TRUE; } +/* Put a constant value in a register. These will not be cleared by + * free_register_overhead. */ static gboolean put_fvalue(dfilter_t *df, fvalue_t *fv, int reg) { df->registers[reg] = g_list_append(NULL, fv); + df->owns_memory[reg] = FALSE; return TRUE; } @@ -396,8 +401,15 @@ any_in_range(dfilter_t *df, int reg1, int reg2, int reg3) } -/* Free the list nodes w/o freeing the memory that each - * list node points to. */ +static void +free_owned_register(gpointer data, gpointer user_data _U_) +{ + fvalue_t *value = (fvalue_t *)data; + FVALUE_FREE(value); +} + +/* Clear registers that were populated during evaluation (leaving constants + * intact). If we created the values, then these will be freed as well. */ static void free_register_overhead(dfilter_t* df) { @@ -406,6 +418,10 @@ free_register_overhead(dfilter_t* df) for (i = 0; i < df->num_registers; i++) { df->attempted_load[i] = FALSE; if (df->registers[i]) { + if (df->owns_memory[i]) { + g_list_foreach(df->registers[i], free_owned_register, NULL); + df->owns_memory[i] = FALSE; + } g_list_free(df->registers[i]); df->registers[i] = NULL; } @@ -437,6 +453,7 @@ mk_range(dfilter_t *df, int from_reg, int to_reg, drange_t *d_range) } df->registers[to_reg] = to_list; + df->owns_memory[to_reg] = TRUE; } @@ -499,6 +516,8 @@ dfvm_apply(dfilter_t *df, proto_tree *tree) } accum = arg1->value.funcdef->function(param1, param2, &df->registers[arg2->value.numeric]); + // functions create a new value, so own it. + df->owns_memory[arg2->value.numeric] = TRUE; break; case MK_RANGE: |