Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 2017] VoIP trace crashes Wireshark when specific RTP Player buttons are clicke

[bugzilla-daemon@xxxxxxxxxxxxx]

http://bugs.wireshark.org/bugzilla/show_bug.cgi?id=2017


jyoung@xxxxxxx changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
Attachment #1327 is|0                           |1
           obsolete|                            |
Attachment #1328 is|0                           |1
           obsolete|                            |
   Attachment #1344|                            |review_for_checkin?
               Flag|                            |




------- Comment #28 from jyoung@xxxxxxx  2008-01-07 07:40 GMT -------
Created an attachment (id=1344)
 --> (http://bugs.wireshark.org/bugzilla/attachment.cgi?id=1344&action=view)
Bug 2017 fix - option 1 version 2 - after g_free() set pointer to NULL

OK, there's more than one bug here...

There's the problem of the stale pointer that can cause Wireshark to crash but
there's also a more subtle "state" problem when RTP_packet_draw() is called
multiple times during "TAP" processing.

Under certain circumstances the gtk/voip_calls.c function RTP_packet_draw() may
be invoked multiple times during "TAP" processing.  If and when this happens,
there is a chance the rtp_listinfo->pt_str item will have a stale pointer.  A
subsequent call to RTP_packet_draw() MAY attempt to use and subsequently
g_free() the stale pointer.  This can lead to bizarre text labels in various
GUI fields, to the outright crashing of Wireshark.

The newly attached patch does two things.  It sets the rtp_listinfo->pt_str
item to NULL following the g_free().   It also augments the immediately
preceeding use of the rtp_listinfo->pt_str in the 'g_strdup_printf("%s (%s)
%s",' call to print the text "<MISSING>" is and when the rtp_listinfo->pt_str
is NULL.  

This patch appears to resolve the crash problem that initiated the creation of
this bug.

But it is now obvious that there is another bug with the way RTP_packet_draw()
interacts with "TAP" processing.   For this bug to surface it appears some type
of timing related event must happen.  On a "slow" computer, the
RTP_packet_draw() function may sometimes return without having completed all
it's work.  It appears that RTP_packet_draw() can be called multiple times
until it completes its work.  RTP_packet_draw() is most likely to be called
multiple times when the "Refiltering statistics on" window is displayed.   

Running Wireshark in valgrind makes this bug appear because Wireshark runs so
slow.  In valgrind the "Refiltering statistics on" always would appear when
opening the Voip Calls window.   

But it was not nearly so easy to reproduce the bug running Wireshark in gdb.  

Putting various watchpoints and breakpoints in gdb would occasionally, but not
consistantly, cause RTP_packet_draw() to run multiple times.  But when
RTP_packet_draw() ran mutiple times it appeared that the "Refiltering
statistics on" also appeared.

So I set about figuring out how to could get the "TAP" processing mechnism to
want to display the "Refiltering statistics on" window while running Wireshark
in gdb.  It turns out it is easy, one simply has to set a breakpoint on
"retap_packet". 

So to get multiple RTP_packet_draw() calls in gdb on my linux system I do the
following:

1: Apply the newly uploaded patch and rebuild Wireshark.

   >  patch -p0 <bug2017.option1.v2.fix.patch
   >  make

2: Start gdb:

  > libtool --mode=execute gdb --args ./wireshark 

3: Set a gdb breakpoint for retap_packet and run wireshark:

> (gdb) b retap_packet
> Breakpoint 1 at 0x8070997: file file.c, line 1849.
> (gdb) run
> Starting program: /home/jyoung/projects/wireshark/.libs/lt-wireshark
> [Thread debugging using libthread_db enabled]
> [New Thread 1112738464 (LWP 30595)]


4: Open of the problem trace file (I've been using the smaller pcap file
(#1313: Subset of initial trace file - call #2).

5: Once ths file is read in open the "Voip Calls" window (Statistics -> Voip
Calls).  Once you select this item, the gdb breakpoint should trigger.

> [Switching to Thread 1112738464 (LWP 30595)]
> 
> Breakpoint 1, retap_packet (cf=0x81b32e0, fdata=0x8b02bc8,
>    pseudo_header=0xbfd91004, pd=0xbfd81004 "", argsp=0x0) at file.c:1849
>1849      edt = epan_dissect_new(num_tap_filters != 0, FALSE);
>(gdb) 

At this point Wireshark will have started displaying the "Voip Calls" window
but there will NOT (yet) be any "Refiltering statistics on" window displayed. 

6:  Allow Wireshark to continue running.  

> (gdb) c
> Continuing.
> 
> Breakpoint 1, retap_packet (cf=0x81b32e0, fdata=0x8b02c18,
>   pseudo_header=0xbfd91004, pd=0xbfd81004 "", argsp=0x0) at file.c:1849
> 1849      edt = epan_dissect_new(num_tap_filters != 0, FALSE);
> (gdb)

This will cause Wireshark to stop the second time that "retap_packet" is
called.   At this point the "Refiltering statistics on" should be displayed.   

7: Now allow Wireshark to continue, but this time we will let gdb ignore the
next 9999 calls to "retap_packet".

> (gdb) c 10000
> Will ignore next 9999 crossings of breakpoint 1.  Continuing.

This should be enough calls to allow TAP processing to complete.   The overhead
of gdb keeping up with the retap_packet breakpoint appears to be enough to make
the RTP_packet_draw() to be called several times and which should make the use
of the NULLed rtp_listinfo->pt_str pointer evident.  

7:  Look to see if NULLed rtp_listinfo->pt_str was used

To see if the NULLed rtp_listinfo->pt_str was used look open "Graph Analysis"
window for the call.  If the NULLed rtp_listinfo->pt_str was referenced, the
codec field for one of the entries will have the text "(<MISSING>)".

Interestingly, the actual generated "Graph Alaysis" report varies.  

If RTP_packet_draw() runs once, then call graph report has an apparently
consistent look and all the codec values will have "(g711U)". 

But if RTP_packet_draw() runs multiple times, then some graph components will
have been created multiple times and some of the codec entries will have
"(<MISSING>)".

At this time I believe the newly uploaded patch should be applied to prevent
the stale pointer related crashes.

Perhaps a NEW bug should be opened to deal with the "state" problems associated
with RTP_packet_draw() when it called multiple times during TAP processing.


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