[asterisk-bugs] [Asterisk 0015911]: [patch] Deadlock in channel masquerade handling

[Asterisk Bug Tracker]

A NOTE has been added to this issue. 
====================================================================== 
https://issues.asterisk.org/view.php?id=15911 
====================================================================== 
Reported By:                russell
Assigned To:                dvossel
====================================================================== 
Project:                    Asterisk
Issue ID:                   15911
Category:                   Core/Channels
Reproducibility:            have not tried
Severity:                   block
Priority:                   normal
Status:                     closed
Target Version:             1.6.3.0
Asterisk Version:           SVN 
JIRA:                       SWP-207 
Regression:                 No 
Reviewboard Link:            
SVN Branch (only for SVN checkouts, not tarball releases):  trunk 
SVN Revision (number only!): 219302 
Request Review:              
Resolution:                 fixed
Fixed in Version:           
====================================================================== 
Date Submitted:             2009-09-17 15:50 CDT
Last Modified:              2009-10-08 15:11 CDT
====================================================================== 
Summary:                    [patch] Deadlock in channel masquerade handling
Description: 
In Asterisk trunk and 1.6.2, ast_channels are stored in an astobj2
container, instead of a singly linked list as before.  The hash value is
based on the channel name.  So, during a masquerade, when the channel name
changes, the channel must be removed and reinserted into the hash table
using its new hash value.

This change introduced locking of the channels container where it did not
exist before.  Specifically, it introduces locking in the order of 1) the
ast_channel, and 2) the container.

Many other places in the code use the locking order 1) ast_channel
container, 2) channels, and this is the locking order that must be obeyed.

Some work needs to be put into masquerade handling to resolve this
deadlock and get it to follow the proper locking order.
====================================================================== 

---------------------------------------------------------------------- 
 (0112082) svnbot (reporter) - 2009-10-08 15:11
 https://issues.asterisk.org/view.php?id=15911#c112082 
---------------------------------------------------------------------- 
Repository: asterisk
Revision: 222761

U   trunk/channels/chan_misdn.c
U   trunk/channels/chan_sip.c
U   trunk/include/asterisk/channel.h
U   trunk/main/channel.c
U   trunk/main/features.c
U   trunk/main/pbx.c

------------------------------------------------------------------------
r222761 | dvossel | 2009-10-07 17:58:38 -0500 (Wed, 07 Oct 2009) | 35
lines

Deadlock in channel masquerade handling

Channels are stored in an ao2_container.  When accessing an item within
an ao2_container the proper locking order is to first lock the container,
and then the items within it.

In ast_do_masquerade both the clone and original channel must be locked
for the entire duration of the function.  The problem with this is that
it attemptes to unlink and link these channels back into the ao2_container
when one of the channel's name changes.  This is invalid locking order as
the process of unlinking and linking will lock the ao2_container while
the channels are locked!!! Now, both the channels in do_masquerade are
unlinked from the ao2_container and then locked for the entire function.
At the end of the function both channels are unlocked and linked back
into the container with their new names as hash values.

This new method of requiring all channels and tech pvts to be unlocked
before ast_do_masquerade() or ast_change_name() required several
changes throughout the code base.

(closes issue https://issues.asterisk.org/view.php?id=15911)
Reported by: russell
Patches:
      masq_deadlock_trunk.diff uploaded by dvossel (license 671)
Tested by: dvossel, atis

(closes issue https://issues.asterisk.org/view.php?id=15618)
Reported by: lmsteffan
Patches:
      deadlock_local_attended_transfers_trunk.diff uploaded by dvossel
(license 671)
Tested by: lmsteffan, dvossel

Review: https://reviewboard.asterisk.org/r/387/


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

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

Issue History 
Date Modified    Username       Field                    Change               
====================================================================== 
2009-10-08 15:11 svnbot         Checkin                                      
2009-10-08 15:11 svnbot         Note Added: 0112082                          
======================================================================