diff options
author | Peter Wu <peter@lekensteyn.nl> | 2019-01-25 18:45:54 +0100 |
---|---|---|
committer | Stig Bjørlykke <stig@bjorlykke.org> | 2019-01-26 19:43:05 +0000 |
commit | 141f6d8df954c990be98d70d8074498688f4a2d3 (patch) | |
tree | 8f9f972e824cbf7f65b2bdbbf976887c278934fe /epan/wslua | |
parent | 14d5ab01c0a0486272d65d3f592e8aed309054dd (diff) |
wslua_field: fix memory leaks in Field_new
Change the "Field" type to actually point to a structure. Do not cheat
and overload the pointer to mean "char*" in one context, and
"header_field_info*" in another. It was very confusing.
Implement Field__gc to free the Field structure that was allocated in
Field_new. This fixes the memory leak in Field_new.
Now the test_wslua_field test passes when executed with ASAN and a bunch
of other wslua tests also improve.
Change-Id: Ibc4318b76bb893151fd40c3fbc595402fba7a60a
Reviewed-on: https://code.wireshark.org/review/31743
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>
Diffstat (limited to 'epan/wslua')
-rw-r--r-- | epan/wslua/wslua.h | 8 | ||||
-rw-r--r-- | epan/wslua/wslua_field.c | 79 |
2 files changed, 47 insertions, 40 deletions
diff --git a/epan/wslua/wslua.h b/epan/wslua/wslua.h index eb6229fd6e..92c7a9318a 100644 --- a/epan/wslua/wslua.h +++ b/epan/wslua/wslua.h @@ -217,6 +217,12 @@ struct _wslua_treeitem { gboolean expired; }; +// Internal structure for wslua_field.c to track info about registered fields. +struct _wslua_header_field_info { + char *name; + header_field_info *hfi; +}; + struct _wslua_field_info { field_info *ws_fi; gboolean expired; @@ -320,7 +326,7 @@ typedef address* Address; typedef nstime_t* NSTime; typedef gint64 Int64; typedef guint64 UInt64; -typedef header_field_info** Field; +typedef struct _wslua_header_field_info* Field; typedef struct _wslua_field_info* FieldInfo; typedef struct _wslua_tap* Listener; typedef struct _wslua_tw* TextWindow; diff --git a/epan/wslua/wslua_field.c b/epan/wslua/wslua_field.c index 5ae3e12661..266eea939b 100644 --- a/epan/wslua/wslua_field.c +++ b/epan/wslua/wslua_field.c @@ -515,6 +515,7 @@ WSLUA_CLASS_DEFINE(Field,FAIL_ON_NULL("Field")); Once created, it is used *inside* the callback functions, to generate a `FieldInfo` object. */ +/* Array of Field (struct _wslua_header_field_info*) pointers.*/ static GPtrArray* wanted_fields = NULL; static dfilter_t* wslua_dfilter = NULL; @@ -555,20 +556,14 @@ void lua_prime_all_fields(proto_tree* tree _U_) { for(i=0; i < wanted_fields->len; i++) { Field f = (Field)g_ptr_array_index(wanted_fields,i); - gchar* name = *((gchar**)f); - *f = proto_registrar_get_byname(name); - - if (!*f) { - report_failure("Could not find field `%s'",name); - *f = NULL; - g_free(name); + f->hfi = proto_registrar_get_byname(f->name); + if (!f->hfi) { + report_failure("Could not find field `%s'", f->name); continue; } - g_free(name); - - g_string_append_printf(fake_tap_filter," || %s",(*f)->abbrev); + g_string_append_printf(fake_tap_filter, " || %s", f->hfi->abbrev); fake_tap = TRUE; } @@ -612,10 +607,10 @@ WSLUA_CONSTRUCTOR Field_new(lua_State *L) { return 0; } - f = (Field)g_malloc(sizeof(void*)); - *f = (header_field_info*)(void*)g_strdup(name); /* cheating */ + f = (Field)g_new0(struct _wslua_header_field_info, 1); + f->name = g_strdup(name); - g_ptr_array_add(wanted_fields,f); + g_ptr_array_add(wanted_fields, f); pushField(L,f); WSLUA_RETURN(1); /* The field extractor */ @@ -653,27 +648,22 @@ WSLUA_CONSTRUCTOR Field_list(lua_State *L) { WSLUA_RETURN(1); /* The array table of field filter names */ } -/* the following is used in Field_get_xxx functions later */ +/* the following is used in Field_get_xxx functions later. If called early + * (wanted_fields is not NULL), it will try to retrieve information directly. + * Otherwise it uses a cached field that was loaded in lua_prime_all_fields. */ #define GET_HFINFO_MEMBER(luafunc, member) \ if (wanted_fields) { \ - /* before registration, so it's a gchar** of the abbrev */ \ - const gchar* name = (const gchar*) *fi; \ - if (name) { \ - hfinfo = proto_registrar_get_byname(name); \ - if (!hfinfo) { \ - /* could be a Lua-created field */ \ - ProtoField pf = wslua_is_field_available(L, name); \ - if (pf) { \ - luafunc(L, pf->member); \ - return 1; \ - } \ + hfinfo = proto_registrar_get_byname(f->name); \ + if (!hfinfo) { \ + /* could be a Lua-created field */ \ + ProtoField pf = wslua_is_field_available(L, f->name); \ + if (pf) { \ + luafunc(L, pf->member); \ + return 1; \ } \ - } else { \ - luaL_error(L, "Field." #member ": unknown field"); \ - return 0; \ } \ } else { \ - hfinfo = *fi; \ + hfinfo = f->hfi; \ } \ \ if (hfinfo) { \ @@ -687,7 +677,7 @@ WSLUA_CONSTRUCTOR Field_list(lua_State *L) { @since 1.99.8 */ static int Field_get_name(lua_State* L) { - Field fi = checkField(L,1); + Field f = checkField(L,1); header_field_info* hfinfo = NULL; GET_HFINFO_MEMBER(lua_pushstring, abbrev); @@ -700,7 +690,7 @@ static int Field_get_name(lua_State* L) { @since 1.99.8 */ static int Field_get_display(lua_State* L) { - Field fi = checkField(L,1); + Field f = checkField(L,1); header_field_info* hfinfo = NULL; GET_HFINFO_MEMBER(lua_pushstring, name); @@ -713,7 +703,7 @@ static int Field_get_display(lua_State* L) { @since 1.99.8 */ static int Field_get_type(lua_State* L) { - Field fi = checkField(L,1); + Field f = checkField(L,1); header_field_info* hfinfo = NULL; GET_HFINFO_MEMBER(lua_pushnumber, type); @@ -724,7 +714,7 @@ static int Field_get_type(lua_State* L) { WSLUA_METAMETHOD Field__call (lua_State* L) { /* Obtain all values (see `FieldInfo`) for this field. */ Field f = checkField(L,1); - header_field_info* in = *f; + header_field_info* in = f->hfi; int items_found = 0; if (! in) { @@ -756,17 +746,28 @@ WSLUA_METAMETHOD Field__tostring(lua_State* L) { /* Obtain a string with the field filter name. */ Field f = checkField(L,1); - if (wanted_fields) { - lua_pushstring(L,*((gchar**)f)); + if (f->hfi) { + /* If a field was found, return the actual field info. */ + lua_pushstring(L, f->hfi->abbrev); } else { - lua_pushstring(L,(*f)->abbrev); + lua_pushstring(L, f->name); } return 1; } -static int Field__gc(lua_State* L _U_) { - /* do NOT free Field */ +static int Field__gc(lua_State* L) { + Field f = toField(L,1); + if (!f) return 0; + + // If out of scope before lua_prime_all_fields is even called, be sure to + // remove the pointer to avoid a use-after-free. + if (wanted_fields) { + g_ptr_array_remove_fast(wanted_fields, f); + } + + g_free(f->name); + g_free(f); return 0; } @@ -814,7 +815,7 @@ int wslua_deregister_fields(lua_State* L _U_) { } /* - * Editor modelines - http://www.wireshark.org/tools/modelines.html + * Editor modelines - https://www.wireshark.org/tools/modelines.html * * Local variables: * c-basic-offset: 4 |