[asterisk-bugs] [Asterisk 0013014]: [patch] ast_iax2_new() fails to account for a bad situation, deals with another incorrectly

[noreply at bugs.digium.com]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=13014 
====================================================================== 
Reported By:                jpgrayson
Assigned To:                russell
====================================================================== 
Project:                    Asterisk
Issue ID:                   13014
Category:                   Channels/chan_iax2
Reproducibility:            sometimes
Severity:                   major
Priority:                   normal
Status:                     assigned
Asterisk Version:           SVN 
SVN Branch (only for SVN checkouts, not tarball releases):  1.4  
SVN Revision (number only!):  
Disclaimer on File?:        N/A 
Request Review:              
====================================================================== 
Date Submitted:             07-07-2008 15:05 CDT
Last Modified:              07-07-2008 17:34 CDT
====================================================================== 
Summary:                    [patch] ast_iax2_new() fails to account for a bad
situation, deals with another incorrectly
Description: 
ast_iax2_new() does a tricky dance when allocating an ast_channel. There
are two related problems in ast_iax2_new() regarding this channel
allocation.

For reference, here is the offending code:

	ast_mutex_unlock(&iaxsl[callno]);
	tmp = ast_channel_alloc(1, state, ...);
	ast_mutex_lock(&iaxsl[callno]);
	if (!iaxs[callno]) {
		if (tmp) {
			ast_channel_free(tmp);
		}
		ast_mutex_unlock(&iaxsl[callno]);
		return NULL;
	}

The first and most obvious problem in the current implementation is that
there is a case we will unlock callno's mutex prior to returning NULL. This
breaks the protocol for ast_iax2_new() -- users of this function expect
iaxsl[callno] to _always_ be returned locked. When this failure case is
hit, callers of ast_iax2_new() will try to unlock an already unlocked
mutex. Whoops.

The second problem is that this test is incomplete. This code only tests
to see if the chan_iax2_pvt object for callno has become null. A better
test is to check if iaxs[callno] has changed (e.g. i != iaxs[callno]).
There are real-world cases where asterisk is being bombarded with new calls
that this callno may not only be destroyed, but destroyed and reused while
ast_channel_alloc() is being called.
====================================================================== 

---------------------------------------------------------------------- 
 svnbot - 07-07-08 17:34  
---------------------------------------------------------------------- 
Repository: asterisk
Revision: 128795

U   branches/1.4/channels/chan_iax2.c

------------------------------------------------------------------------
r128795 | russell | 2008-07-07 17:34:20 -0500 (Mon, 07 Jul 2008) | 8 lines

Fix handling of when a pvt disappears.  Properly return the pvt locked
and don't hold the pvt lock while destroying the ast_channel.

(closes issue http://bugs.digium.com/view.php?id=13014)
Reported by: jpgrayson
Patches:
      chan_iax2_ast_iax2_new2.patch uploaded by jpgrayson (license 492)

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

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

Issue History 
Date Modified   Username       Field                    Change               
====================================================================== 
07-07-08 17:34  svnbot         Note Added: 0089865                          
======================================================================