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>
This commit is contained in:
Peter Wu 2019-01-25 18:45:54 +01:00 committed by Stig Bjørlykke
parent 14d5ab01c0
commit 141f6d8df9
2 changed files with 47 additions and 40 deletions

View File

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

View File

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