[Home]

Summary:ASTERISK-15694: [patch] [regression] Duplicate TXREQ packets will cause chan_iax2 to reject an unrelated call in the future
Reporter:Ben Winslow (rain)Labels:
Date Opened:2010-02-25 11:32:38.000-0600Date Closed:2010-03-03 12:06:20.000-0600
Priority:MinorRegression?No
Status:Closed/CompleteComponents:Channels/chan_iax2
Versions:Frequency of
Occurrence
Related
Issues:
Environment:Attachments:( 0) iax2-double-txreq-fix.diff
Description:When Asterisk receives an IAX2 TXREQ packet, try_transfer() will call store_by_transfercallno() to link the chan_iax2_pvt struct into iax_transfercallno_pvts.  If a duplicate TXREQ packet is received for the same call, the pvt struct will be linked into iax_transfercallno_pvts multiple times.

Since store_by_transfercallno() is little more than a wrapper for ao2_link(), it will dutifully add the chan_iax2_pvt struct to iax_transfercallno_pvts each time it's called; however, the call will only be unlinked once (in iax2_destroy() or complete_transfer().¹)  This results in a memory leak (Asterisk sets the pointer in iaxs[] to NULL in iax2_destroy() and will never unreference this chan_iax2_pvt again) and, eventually, a rejected call.

After the call where the duplicate TXREQs were received ends, the originating host may reuse the same call number.  When this call number appears in a NEW packet, find_callno() will find the old chan_iax2_pvt in iax_transfercallno_pvts and return its callno.  Since the iaxs[] entry for this callno is NULL (the call was probably destroyed long ago), socket_process() will summarily reject the new call with INVAL via raw_hangup().

My fix (attached) is to only call store_by_transfercallno() if pvt->transferring is TRANSFER_NONE immediately before ->transferring is set to TRANSFER_BEGIN.  The structure will still be updated if the transferid is changed in the duplicate TXREQ, so I don't believe this will cause any issues.

I've been testing this patch for over 12 hours and haven't noticed any issues yet.

¹ Actually, this is a lie, due to a trivial bug in complete_transfer(): pvt->transfercallno should be set to 0 when the transfer completes, not -1.  A fix for this is also in the patch I've attached.


****** STEPS TO REPRODUCE ******

This is a bit difficult to reproduce on demand, since it depends on the duplicate TXREQ, the transfer failing, AND the call number being reused later.  When the problem occurs, no message is logged on the end rejecting the call, and the calling end's Dial() will quietly fail with a DIALSTATUS of CHANUNAVAIL.

****** ADDITIONAL INFORMATION ******

It looks like this bug was introduced in Asterisk 1.4.24, 1.6.0.6, and 1.6.1.0-rc2.
Comments:By: Leif Madsen (lmadsen) 2010-02-25 12:42:17.000-0600

Do you happen to know which revision number prior to those releases this could have been?
By: Ben Winslow (rain) 2010-02-25 14:20:12.000-0600

Sure, these are all related to the fix for issue ASTERISK-12718:
1.4 - r173248
1.6.0 - r173250
1.6.1 - r173506
By: David Vossel (dvossel) 2010-03-03 11:31:06.000-0600

wow, this is really a great find.  I'll test the patch as well just to make sure before committing it, but this patch looks exactly right.  Thanks!
By: Digium Subversion (svnbot) 2010-03-03 12:02:28.000-0600

Repository: asterisk
Revision: 250394

U   branches/1.4/channels/chan_iax2.c

------------------------------------------------------------------------
r250394 | dvossel | 2010-03-03 12:02:27 -0600 (Wed, 03 Mar 2010) | 16 lines

fixes problem with duplicate TXREQ packets

When Asterisk receives an IAX2 TXREQ packet, try_transfer()
will call store_by_transfercallno() to link the chan_iax2_pvt
struct into iax_transfercallno_pvts. If a duplicate TXREQ
packet is received for the same call, the pvt struct will be
linked into iax_transfercallno_pvts multiple times.  This patch
fixes this.  Thanks rain for debugging this and providing a patch!

(closes issue ASTERISK-15694)
Reported by: rain
Patches:
     iax2-double-txreq-fix.diff uploaded by rain (license 327)
Tested by: rain, dvossel


------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=250394
By: Digium Subversion (svnbot) 2010-03-03 12:03:21.000-0600

Repository: asterisk
Revision: 250395

_U  trunk/
U   trunk/channels/chan_iax2.c

------------------------------------------------------------------------
r250395 | dvossel | 2010-03-03 12:03:21 -0600 (Wed, 03 Mar 2010) | 22 lines

Merged revisions 250394 via svnmerge from
https://origsvn.digium.com/svn/asterisk/branches/1.4

........
 r250394 | dvossel | 2010-03-03 12:02:27 -0600 (Wed, 03 Mar 2010) | 16 lines
 
 fixes problem with duplicate TXREQ packets
 
 When Asterisk receives an IAX2 TXREQ packet, try_transfer()
 will call store_by_transfercallno() to link the chan_iax2_pvt
 struct into iax_transfercallno_pvts. If a duplicate TXREQ
 packet is received for the same call, the pvt struct will be
 linked into iax_transfercallno_pvts multiple times.  This patch
 fixes this.  Thanks rain for debugging this and providing a patch!
 
 (closes issue ASTERISK-15694)
 Reported by: rain
 Patches:
       iax2-double-txreq-fix.diff uploaded by rain (license 327)
 Tested by: rain, dvossel
........

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=250395
By: Digium Subversion (svnbot) 2010-03-03 12:05:01.000-0600

Repository: asterisk
Revision: 250396

_U  branches/1.6.2/
U   branches/1.6.2/channels/chan_iax2.c

------------------------------------------------------------------------
r250396 | dvossel | 2010-03-03 12:05:01 -0600 (Wed, 03 Mar 2010) | 29 lines

Merged revisions 250395 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r250395 | dvossel | 2010-03-03 12:03:19 -0600 (Wed, 03 Mar 2010) | 22 lines
 
 Merged revisions 250394 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r250394 | dvossel | 2010-03-03 12:02:27 -0600 (Wed, 03 Mar 2010) | 16 lines
   
   fixes problem with duplicate TXREQ packets
   
   When Asterisk receives an IAX2 TXREQ packet, try_transfer()
   will call store_by_transfercallno() to link the chan_iax2_pvt
   struct into iax_transfercallno_pvts. If a duplicate TXREQ
   packet is received for the same call, the pvt struct will be
   linked into iax_transfercallno_pvts multiple times.  This patch
   fixes this.  Thanks rain for debugging this and providing a patch!
   
   (closes issue ASTERISK-15694)
   Reported by: rain
   Patches:
         iax2-double-txreq-fix.diff uploaded by rain (license 327)
   Tested by: rain, dvossel
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=250396
By: Digium Subversion (svnbot) 2010-03-03 12:05:24.000-0600

Repository: asterisk
Revision: 250397

_U  branches/1.6.1/
U   branches/1.6.1/channels/chan_iax2.c

------------------------------------------------------------------------
r250397 | dvossel | 2010-03-03 12:05:24 -0600 (Wed, 03 Mar 2010) | 29 lines

Merged revisions 250395 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r250395 | dvossel | 2010-03-03 12:03:19 -0600 (Wed, 03 Mar 2010) | 22 lines
 
 Merged revisions 250394 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r250394 | dvossel | 2010-03-03 12:02:27 -0600 (Wed, 03 Mar 2010) | 16 lines
   
   fixes problem with duplicate TXREQ packets
   
   When Asterisk receives an IAX2 TXREQ packet, try_transfer()
   will call store_by_transfercallno() to link the chan_iax2_pvt
   struct into iax_transfercallno_pvts. If a duplicate TXREQ
   packet is received for the same call, the pvt struct will be
   linked into iax_transfercallno_pvts multiple times.  This patch
   fixes this.  Thanks rain for debugging this and providing a patch!
   
   (closes issue ASTERISK-15694)
   Reported by: rain
   Patches:
         iax2-double-txreq-fix.diff uploaded by rain (license 327)
   Tested by: rain, dvossel
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=250397
By: Digium Subversion (svnbot) 2010-03-03 12:06:19.000-0600

Repository: asterisk
Revision: 250398

_U  branches/1.6.0/
U   branches/1.6.0/channels/chan_iax2.c

------------------------------------------------------------------------
r250398 | dvossel | 2010-03-03 12:06:19 -0600 (Wed, 03 Mar 2010) | 29 lines

Merged revisions 250395 via svnmerge from
https://origsvn.digium.com/svn/asterisk/trunk

................
 r250395 | dvossel | 2010-03-03 12:03:19 -0600 (Wed, 03 Mar 2010) | 22 lines
 
 Merged revisions 250394 via svnmerge from
 https://origsvn.digium.com/svn/asterisk/branches/1.4
 
 ........
   r250394 | dvossel | 2010-03-03 12:02:27 -0600 (Wed, 03 Mar 2010) | 16 lines
   
   fixes problem with duplicate TXREQ packets
   
   When Asterisk receives an IAX2 TXREQ packet, try_transfer()
   will call store_by_transfercallno() to link the chan_iax2_pvt
   struct into iax_transfercallno_pvts. If a duplicate TXREQ
   packet is received for the same call, the pvt struct will be
   linked into iax_transfercallno_pvts multiple times.  This patch
   fixes this.  Thanks rain for debugging this and providing a patch!
   
   (closes issue ASTERISK-15694)
   Reported by: rain
   Patches:
         iax2-double-txreq-fix.diff uploaded by rain (license 327)
   Tested by: rain, dvossel
 ........
................

------------------------------------------------------------------------

http://svn.digium.com/view/asterisk?view=rev&revision=250398