[Asterisk-code-review] Prevent deadlock when performing BYE with Also transfer. (asterisk[11])

[Mark Michelson]

Mark Michelson has uploaded a new change for review.

  https://gerrit.asterisk.org/644

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

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(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/44/644/1

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: newchange
Gerrit-Change-Id: I72c4fc295ec8573bee599e8e9213c5350a3cd224
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Mark Michelson <mmichelson at digium.com>