[asterisk-commits] mmichelson: trunk r377911 - in /trunk: ./ channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: mmichelson
Date: Tue Dec 11 18:02:31 2012
New Revision: 377911

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=377911
Log:
Fix a potential deadlock in chan_sip during transfers.

The issue comes from the fact that transfers may perform
a redirecting update on a channel. The issue is that lock
inversion between the channel and its tech_pvt occurs since
the channel lock is released during the transfer process.

The fix is to move when the redirecting update occurs to a
place where neither the tech_pvt or the channel is locked so
that the two can be locked in the proper order.

(closes issue ASTERISK-20708)
reported by Mark Michelson
patches:
	ASTERISK-20708-3.patch uploaded by Mark Michelson (License #5049)

Tested by:
	Tim Ringenbach at Asteria Solutions Group
........

Merged revisions 377910 from http://svn.asterisk.org/svn/asterisk/branches/11

Modified:
    trunk/   (props changed)
    trunk/channels/chan_sip.c

Propchange: trunk/
------------------------------------------------------------------------------
Binary property 'branch-11-merged' - no diff available.

Modified: trunk/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/trunk/channels/chan_sip.c?view=diff&rev=377911&r1=377910&r2=377911
==============================================================================
--- trunk/channels/chan_sip.c (original)
+++ trunk/channels/chan_sip.c Tue Dec 11 18:02:31 2012
@@ -26331,6 +26331,24 @@
 		if (!ast_strlen_zero(referred_by)) {
 			pbx_builtin_setvar_helper(current.chan2, "_SIPTRANSFER_REFERER", referred_by);
 		}
+
+		/* When a call is transferred to voicemail from a Digium phone, there may be
+		 * a Diversion header present in the REFER with an appropriate reason parameter
+		 * set. We need to update the redirecting information appropriately.
+		 */
+		ast_channel_lock(p->owner);
+		sip_pvt_lock(p);
+		ast_party_redirecting_init(&redirecting);
+		memset(&update_redirecting, 0, sizeof(update_redirecting));
+		change_redirecting_information(p, req, &redirecting, &update_redirecting, FALSE);
+
+		/* Do not hold the pvt lock during a call that causes an indicate or an async_goto.
+		 * Those functions lock channels which will invalidate locking order if the pvt lock
+		 * is held.*/
+		sip_pvt_unlock(p);
+		ast_channel_unlock(p->owner);
+		ast_channel_update_redirecting(current.chan2, &redirecting, &update_redirecting);
+		ast_party_redirecting_free(&redirecting);
 	}
 
 	sip_pvt_lock(p);
@@ -26378,20 +26396,7 @@
 	}
 	ast_set_flag(&p->flags[0], SIP_DEFER_BYE_ON_TRANSFER);	/* Delay hangup */
 
-	/* When a call is transferred to voicemail from a Digium phone, there may be
-	 * a Diversion header present in the REFER with an appropriate reason parameter
-	 * set. We need to update the redirecting information appropriately.
-	 */
-	ast_party_redirecting_init(&redirecting);
-	memset(&update_redirecting, 0, sizeof(update_redirecting));
-	change_redirecting_information(p, req, &redirecting, &update_redirecting, FALSE);
-
-	/* Do not hold the pvt lock during a call that causes an indicate or an async_goto.
-	 * Those functions lock channels which will invalidate locking order if the pvt lock
-	 * is held.*/
 	sip_pvt_unlock(p);
-	ast_channel_update_redirecting(current.chan2, &redirecting, &update_redirecting);
-	ast_party_redirecting_free(&redirecting);
 
 	/* For blind transfers, move the call to the new extensions. For attended transfers on multiple
 	 * servers - generate an INVITE with Replaces. Either way, let the dial plan decided