EMEMification

This is the temporary page for coordinating EMEMification of (almost) all memory management in Wireshark dissectors. Once this job is done this page will be deleted.

Below are lists of items that need to be addressed. Once you have finished auditing and maybe refactoring a specific file, please update the file lists accordingly.

g_malloc()

A lot of g_malloc() usage in the dissectors is unsafe and contain either obvious memory leaks or non obvious ones such as allocating a blob and never releasing them due to an exception being raised.

Most g_malloc() calls inside the dissectors are for memory that are either just for packet scope, i.e. can be safely thrown away after the packet dissection has completed, or for memory containing state that has to remain until the next capture file is read. Few allocations have different scope.

To proceed here, audit the code, follow the allocation, assignments and when and where the objects are used/freed (if they are freed at all). Once you know for sure that the allocation is either for packet lifetime (use ep_alloc()) or capturefile lifetime (use se_alloc()) fix the file and update the list below. If you are unsure, just leave the file, if you are sure that allocation is a bit hairier than the two modes described above, move the file to the "fix later" section.

See doc/README.malloc, epan/emem.h and epan/emem.c for more details about the allocation functions and the allocation scope. Grep for ep_alloc() in epan/dissectors for examples on how it can be used.

To use the new memory allocation functions you have to #include <epan/emem.h> . For replacing g_malloc() there are two functions there of special interest :

ep_alloc() that allocates memory that will automatically be released once packet dissection ends.

se_alloc() that allocates memory that will automatically be released once a new/different capture file is loaded. This function is likely only called when pinfo->fd->flags.visited==0, if not, then there is more than likely something broken with the code and you will have to be extra careful when auditing.

These functions can not free() individual blocks of memory at a time, it is all or nothing so if neither of release-everything-after-dissecting-the-packet or release-everything-when-loading-a-new-file fit the useage of the memory allocation, you might be better off not to ememify the use.

If g_malloc()/tvb_memdup() is used to allocate data for a new tvb attached to the main tvb using tvb_new_real_data()/tvb_set_child_real_data_tvbuff() then we can not replace g_malloc() with ep_alloc() since tvbs' are not released when the packet dissection has ended. Instead they are released after the NEXT packet has been dissected, so for those ones, just make sure that an explicit free() callback is registered for the new tvb using tvb_set_free_cb(new_tvb, g_free).

Not yet audited

packet-acse-template.c (g_strdup)
packet-aim.c (g_new)
packet-camel-template.c (g_strdup)
camel.cnf (calloc)
packet-giop.c (g_new0 & g_malloc)
packet-gsm_map-template.c (g_malloc, g_strdup)
packet-gssapi.c
packet-http.c
packet-iax2.c
packet-ieee80211.c
packet-ieee802a.c
packet-kerberos.c
packet-ldap.c
packet-mount.c
packet-multipart.c
packet-netflow.c
packet-nfs.c
packet-nlm.c
packet-ntlmssp.c
packet-ppp.c
packet-radius.c
packet-rpc.c
packet-rsync.c
packet-sigcomp.c
packet-smb.c
packet-spnego.c
packet-tacacs.c
packet-vj.c
packet-wbxml.c
packet-wsp.c
packet-x11.c
packet-xdmcp.c

Audited but contain non obvious allocation scope, leave for later

packet-icq.c    g_malloc()s data for a tvb.   tvb_set_free_cb() confirmed.
packet-telnet.c g_malloc()s data for a tvb.   tvb_set_free_cb() added in r15361.

g_strdup()/g_strdup_printf()

Not yet audited

packet-acse.c
packet-bssgp.c
packet-csm-encaps.c
packet-dcerpc-lsa.c
packet-dcerpc-nt.c
packet-dcerpc-samr.c
packet-dcerpc-spoolss.c
packet-dcerpc-svcctl.c
packet-dcm.c
packet-diameter.c
packet-giop.c
packet-gsm_map.c
packet-gssapi.c
packet-http.c
packet-icep.c
packet-jxta.c
packet-k12.c
packet-ldap.c
packet-mmse.c
packet-mq.c
packet-msrp.c
packet-multipart.c
packet-nfs.c
packet-quakeworld.c
packet-radius.c
packet-smb-pipe.c
packet-smb-sidsnooping.c
packet-wbxml.c
packet-windows-common.c
packet-wsp.c

Audited but contain non obvious allocation scope, leave for later

sprintf()

Not yet audited

addr_resolv.c
except.c
inet_ntop.c
osi-utils.c
prefs.c
proto.c
radius_dict.c
range.c
sha1.c
sna-utils.c
stats_tree.c
tap.c
to_str.c
value_string.c
xdlc.c
packet-aarp.c
packet-ajp13.c
packet-ansi_637.c
packet-ansi_a.c
packet-ansi_map.c
packet-armagetronad.c
packet-bgp.c
packet-bootp.c
packet-clnp.c
packet-cops.c
packet-dcm.c
packet-diameter.c
packet-dns.c
packet-fcdns.c
packet-fcels.c
packet-fcswils.c
packet-fddi.c
packet-fw1.c
packet-gsm_a.c
packet-h245.c
packet-iax2.c
packet-icep.c
packet-icmpv6.c
packet-ieee80211.c
packet-ip.c
packet-isis-clv.c
packet-kerberos.c
packet-nbns.c
packet-per.c
packet-radius.c
packet-ranap.c
packet-smb-sidsnooping.c
packet-tds.c
packet-windows-common.c
packet-wtp.c

Audited but contain non obvious allocation scope, leave for later

Discussion

Anyone want to add more text or translate into real English?

There are some more GLib/GTK functions which internally call g_malloc(), these come to my mind:

... and obviously the GTK widgets will use heap memory too. - UlfLamping

Unfortunately, the use of g_malloc() is a "feature" of GTK+ - and it means that, even if we were to eliminate all use of g_malloc() (and callers of it) outside the GUI code, we'd still run the risk of g_malloc() crashing the application with abort() if it runs out of memory. That might not be a problem with other GUI toolkits.

Perhaps this is adressed in GTK 2.8 "* g_try_new(), g_try_renew(), g_try_new0() and g_try_malloc0()

- Anders Broman

A big problem is that the GtkCList widget allocates strings for every column value for every row of the display, and, when reading in a capture file, that could consume a lot of memory. Displaying the packet list with a widget that calls back to a routine to get the column values could help this, although to avoid making copies of any of the columns, we'd have to support efficient random access to gzipped files, so that the columns can be generated reasonably quickly on the fly. Some of that, but not the compressed file issue, is mentioned in Development/Wishlist. -Guy Harris

We might also have a look at the remaining traditional malloc calls (some are still left): malloc / calloc / realloc / valloc / memalign / free / strdup - UlfLamping

EMEMification (last edited 2008-04-12 17:50:16 by localhost)