[asterisk-commits] mjordan: branch 11 r383878 - in /branches/11: ./ channels/chan_sip.c

[SVN commits to the Asterisk project]

Author: mjordan
Date: Mon Mar 25 21:28:31 2013
New Revision: 383878

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=383878
Log:
Resolve deadlock between SIP registration and channel based functions

In r373424, several reentrancy problems in chan_sip were addressed. As a
result, the SIP channel driver is now properly locking the channel driver
private information in certain operations that it wasn't previously. This
exposed two latent problems either in register_verify or by functions called
by register_verify. This includes:
 * Holding the private lock while calling sip_send_mwi_to_peer. This can create
   a new sip_pvt via sip_alloc, which will obtain the channel container lock.
   This is a locking inversion, as any channel related lock must be obtained
   prior to obtaining the SIP channel technology private lock.

   Note that this issue was already fixed in Asterisk 11.

 * Holding the private lock while calling sip_poke_peer. In the same vein as
   sip_send_mwi_to_peer, sip_poke_peer can create a new SIP private, causing
   the same locking inversion.

Note that this locking inversion typically occured when CLI commands were run
while a SIP REGISTER request was being processed, as many CLI commands (such
as 'sip show channels', 'core show channels', etc.) have to obtain the channel
container lock.

(issue ASTERISK-21068)
Reported by: Nicolas Bouliane

(issue ASTERISK-20550)
Reported by: David Brillert

(issue ASTERISK-21314)
Reported by: Badalian Vyacheslav

(issue ASTERISK-21296)
Reported by: Gabriel Birke
........

Merged revisions 383863 from http://svn.asterisk.org/svn/asterisk/branches/1.8

Modified:
    branches/11/   (props changed)
    branches/11/channels/chan_sip.c

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

Modified: branches/11/channels/chan_sip.c
URL: http://svnview.digium.com/svn/asterisk/branches/11/channels/chan_sip.c?view=diff&rev=383878&r1=383877&r2=383878
==============================================================================
--- branches/11/channels/chan_sip.c (original)
+++ branches/11/channels/chan_sip.c Mon Mar 25 21:28:31 2013
@@ -16025,7 +16025,9 @@
 		ast_verb(3, "Registered SIP '%s' at %s\n", peer->name,
 			ast_sockaddr_stringify(&peer->addr));
 	}
+	sip_pvt_unlock(pvt);
 	sip_poke_peer(peer, 0);
+	sip_pvt_lock(pvt);
 	register_peer_exten(peer, 1);
 
 	/* Save User agent */
@@ -16918,9 +16920,9 @@
 	}
 	if (!res) {
 		if (send_mwi) {
-			ao2_unlock(p);
+			sip_pvt_unlock(p);
 			sip_send_mwi_to_peer(peer, 0);
-			ao2_lock(p);
+			sip_pvt_lock(p);
 		} else {
 			update_peer_lastmsgssent(peer, -1, 0);
 		}
@@ -29273,6 +29275,9 @@
 \note	This is done with 60 seconds between each ping,
 	unless forced by cli or manager. If peer is unreachable,
 	we check every 10th second by default.
+\note Do *not* hold a pvt lock while calling this function.
+	This function calls sip_alloc, which can cause a deadlock
+	if another sip_pvt is held.
 */
 static int sip_poke_peer(struct sip_peer *peer, int force)
 {