forked from osmocom/wireshark
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>
This commit is contained in:
parent
0de109ef57
commit
6144951380
|
@ -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;
|
||||
|
|
|
@ -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);
|
||||
|
|
|
@ -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:
|
||||
|
|
|
@ -123,3 +123,11 @@ class testIPv4(dftest.DFTest):
|
|||
def test_slice_4(self):
|
||||
dfilter = "ip.src[2:2] == ff:ff"
|
||||
self.assertDFilterCount(dfilter, 0)
|
||||
|
||||
def test_count_1(self):
|
||||
dfilter = "count(ip.src) == 1"
|
||||
self.assertDFilterCount(dfilter, 2)
|
||||
|
||||
def test_count_2(self):
|
||||
dfilter = "count(ip.addr) == 2"
|
||||
self.assertDFilterCount(dfilter, 2)
|
||||
|
|
|
@ -156,7 +156,10 @@ class testString(dftest.DFTest):
|
|||
dfilter = 'lower(http.user_agent) contains "update"'
|
||||
self.assertDFilterCount(dfilter, 1)
|
||||
|
||||
def test_contains_lower_2(self):
|
||||
def test_eq_lower_1(self):
|
||||
dfilter = 'lower(tcp.seq) == 4'
|
||||
self.assertDFilterFail(dfilter)
|
||||
|
||||
def test_string_len(self):
|
||||
dfilter = 'len(http.request.method) == 4'
|
||||
self.assertDFilterCount(dfilter, 1)
|
||||
|
|
Loading…
Reference in New Issue