[asterisk-bugs] [Asterisk 0016904]: [patch] [regression] Duplicate TXREQ packets will cause chan_iax2 to reject an unrelated call in the future

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
https://issues.asterisk.org/view.php?id=16904 
====================================================================== 
Reported By:                rain
Assigned To:                dvossel
====================================================================== 
Project:                    Asterisk
Issue ID:                   16904
Category:                   Channels/chan_iax2
Reproducibility:            random
Severity:                   minor
Priority:                   normal
Status:                     closed
Asterisk Version:           1.6.2.2 
JIRA:                       SWP-967 
Regression:                 No 
Reviewboard Link:            
SVN Branch (only for SVN checkouts, not tarball releases): N/A 
SVN Revision (number only!):  
Request Review:              
Resolution:                 fixed
Fixed in Version:           
====================================================================== 
Date Submitted:             2010-02-25 11:32 CST
Last Modified:              2010-03-03 12:06 CST
====================================================================== 
Summary:                    [patch] [regression] Duplicate TXREQ packets will
cause chan_iax2 to reject an unrelated call in the future
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.

====================================================================== 

---------------------------------------------------------------------- 
 (0118876) svnbot (reporter) - 2010-03-03 12:06
 https://issues.asterisk.org/view.php?id=16904#c118876 
---------------------------------------------------------------------- 
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 https://issues.asterisk.org/view.php?id=16904)
    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 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2010-03-03 12:06 svnbot         Checkin                                      
2010-03-03 12:06 svnbot         Note Added: 0118876                          
======================================================================