[asterisk-bugs] [Asterisk 0014623]: [patch] Race condition between bridge and channel masquerading

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
http://bugs.digium.com/view.php?id=14623 
====================================================================== 
Reported By:                guillecabeza
Assigned To:                russell
====================================================================== 
Project:                    Asterisk
Issue ID:                   14623
Category:                   Core/Channels
Reproducibility:            sometimes
Severity:                   crash
Priority:                   normal
Status:                     assigned
Asterisk Version:           SVN 
Regression:                 No 
SVN Branch (only for SVN checkouts, not tarball releases):  trunk 
SVN Revision (number only!): 180032 
Request Review:              
====================================================================== 
Date Submitted:             2009-03-08 16:07 CDT
Last Modified:              2009-03-10 08:39 CDT
====================================================================== 
Summary:                    [patch] Race condition between bridge and channel
masquerading
Description: 
The bridge checks for zombieness

/* Stop if we're a zombie or need a soft hangup */
		if (ast_test_flag(c0, AST_FLAG_ZOMBIE) || ast_check_hangup_locked(c0)
||
		    ast_test_flag(c1, AST_FLAG_ZOMBIE) || ast_check_hangup_locked(c1))
{

To see if some of the channels is going through a masquerading, but non
atomically, access to channel member variables happens a few lines later

if (!ast_strlen_zero(pbx_builtin_getvar_helper(c0, "BRIDGEPEER")))
     pbx_builtin_setvar_helper(c0, "BRIDGEPEER", c1->name);

Without locking on those acceses. That causes random crashes when the
memory is touched or free'd later.

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

---------------------------------------------------------------------- 
 (0101463) russell (administrator) - 2009-03-10 08:39
 http://bugs.digium.com/view.php?id=14623#c101463 
---------------------------------------------------------------------- 
Thanks for the feedback, guillecabeza.

I don't think that ast_channel_masquerade() is quite the same situation. 
In this case, we can pretty easily avoid having to hold a lock on 2
channels at the same time.  That lets us avoid the nasty trylock/usleep
loop.  In the case of ast_channel_masquerade() and a number of other places
in the code, it would be much more difficult to avoid holding 2 channel
locks, so we have the nasty deadlock avoidance attempt to get the locks.

I'll go ahead and commit this patch today.  Thanks for your work on this
issue! 

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-03-10 08:39 russell        Note Added: 0101463                          
======================================================================