[asterisk-commits] chan sip: Prevent deadlock when performing BYE with Also tra... (asterisk[11])

[SVN commits to the Asterisk project]

Matt Jordan has submitted this change and it was merged.

Change subject: chan_sip: Prevent deadlock when performing BYE with Also transfer.
......................................................................


chan_sip: Prevent deadlock when performing BYE with Also transfer.

When a BYE with an Also header is successfully processed, and the sender
of the BYE is bridged with another channel, chan_sip will unlock the
owner of the dialog on which the BYE was received, call ast_async_goto()
on the bridged channel, and then re-lock the owner. The reason for this
locking behavior is that ast_async_goto() can result in a masquerade,
which requires that the involved channels are unlocked.

The problem here is that this causes a locking inversion since the
dialog's lock is held when re-locking the owner channel after the async
goto. The lock order is supposed to be channel and then sip_pvt.

The fix proposed is simple. In addition to unlocking the owner channel
before the ast_async_goto() call, also unlock the sip_pvt. Then relock
both after ast_async_goto() returns, being sure to lock the channel and
then the sip_pvt.

ASTERISK-25139 #close
Reported by Gregory Massel

Change-Id: I72c4fc295ec8573bee599e8e9213c5350a3cd224
---
M channels/chan_sip.c
1 file changed, 4 insertions(+), 1 deletion(-)

Approvals:
  Matt Jordan: Looks good to me, approved; Verified
  Joshua Colp: Looks good to me, but someone else must approve



diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 3637ad7..351fa53 100644
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -26977,11 +26977,14 @@
 				if (bridged_to) {
 					/* Don't actually hangup here... */
 					ast_queue_control(c, AST_CONTROL_UNHOLD);
+					sip_pvt_unlock(p);
 					ast_channel_unlock(c);  /* async_goto can do a masquerade, no locks can be held during a masq */
 					ast_async_goto(bridged_to, p->context, p->refer->refer_to, 1);
 					ast_channel_lock(c);
-				} else
+					sip_pvt_lock(p);
+				} else {
 					ast_queue_hangup(p->owner);
+				}
 			}
 		} else {
 			ast_log(LOG_WARNING, "Invalid transfer information from '%s'\n", ast_sockaddr_stringify(&p->recv));

-- 
To view, visit https://gerrit.asterisk.org/644
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: I72c4fc295ec8573bee599e8e9213c5350a3cd224
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>