Wireshark-bugs: [Wireshark-bugs] [Bug 2017] VoIP trace crashes Wireshark when specific RTP Playe
Date Prev · Date Next · Thread Prev · Thread Next
Date: Tue, 1 Jan 2008 08:09:59 +0000 (GMT)
http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2017





------- Comment #19 from jyoung@xxxxxxx  2008-01-01 08:09 GMT -------
Ok, this was a nasty bug, but thanks to Steve's earlier work, valgrind and some
extra instrumentation code I finally figured out what is happening:

The problem is caused by a flaw in the RTP_packet_draw() function.

Specifically it was the following line that ultimately initiated the problems:

  gtk/voip_call.c:634:  g_free(rtp_listinfo->pt_str);

After the g_free() the rtp_listinfo->pt_str retained a stale pointer; the
ponter was never cleared.  The freed area of memory would (sometimes) be
reassigned to something else.  Yet subsequent runs through this particular line
of code would cause the original pointer address (which may or may not have
been reassigned) to be g_free()ed again and again.  Sometimes the same exact
pointer address would be assigned to something else which caused wierd failures
far from the original cause.  At other times the stale pointer would point
inside a (now larger) block of memory.   

I see a two possible "quick" fixes to this bug, but I'm not sure either one is
actually the correct solution.   

One fix (option 1) is to simply NULL the rtp_listinfo->pt_str after the
g_free().  This fix works because attempting to g_free() a NULL pointer is
"safe". (I personally don't like this option.) 

Another fix (option 2) to is to simply delete the offending g_free() line.
Within the context of the RTP_packet_draw() function I can't see why this
particular g_free(rtp_listinfo->pt_str) even exists. (This is my preferred
quick fix.)       

With either fix applied I could no longer recreate the original problem that
started this bug report.  Neither option SEEMS to break anything new.  

But a third option may be warrented.  I suspect that this particular g_free()
may have originally been planned for something like the "newgai->comment" field
but I can't really tell (There is some very similar code at
gtk/voip_call.c:612).

I'll attach two small patches for this bug (option 1) and (option 2).  
Hopefully someone will determine which if either should be applied, or if a
third option should be developed.

FYI:

For what it's worth valgrind definitly helped locate this bug.  Here's how I
launched valgrind from my deveopment directory:

  valgrind \
    --trace-children=yes \
    --leak-check=full \
    --track-fds=yes \
    --num-callers=50 \
    --time-stamp=yes \
    --freelist-vol=100000000 \
    -v \
    ./wireshark

These valgrind flags WILL result in a pretty verbose report, but if you capture
the output to a file (e.g. use "script" prior to starting valgrind) you can
easily review the output.  While working to resolve this bug I believe valgrind
 indicated that there may be other latent bugs in other areas of wireshark (not
counting the lost memory issues).  I expect it will be a worthwhile excercise
to get more comfortable with running wireshark in valgrind.  Some areas where
valgrind "alerted" included when I simply opened the "Wireshark: Open Capture
File" dialog (i.e. the "File -> Open" menu option) and when I had a non-empty
~/.wireshark/snmp_users file.


-- 
Configure bugmail: http://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.