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

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 3637ad74686112db7c27ae613f7605705fb464a5..351fa53cbce2431b2103efa97e8d958159ce8817 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -26977,11 +26977,14 @@ static int handle_request_bye(struct sip_pvt *p, struct sip_request *req)
                                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));