Prevent BLF subscriptions from causing deadlocks

Fix a locking inversion in sip_send_mwi_to_peer that was causing deadlocks.
This function now requires that both the peer and associated pvt be unlocked
before it is called for cases where peer and peer->mwipvt form a circular
reference.

(closes issue ASTERISK-18663)
Review: https://reviewboard.asterisk.org/r/1563/

git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@343621 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 29c7aaff4c84092c2ebee64a79428616aed78672..c84131d5d2d7dd1b02c2ce63ff3c6f5645fd9a3d 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -24345,7 +24345,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
 
                p->subscribed = MWI_NOTIFICATION;
                if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) {
+                       ao2_unlock(p);
                        add_peer_mwi_subs(authpeer);
+                       ao2_lock(p);
                }
                if (authpeer->mwipvt && authpeer->mwipvt != p) {        /* Destroy old PVT if this is a new one */
                        /* We only allow one subscription per peer */
@@ -24421,7 +24423,12 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
                        ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
                        transmit_response(p, "200 OK", req);
                        if (p->relatedpeer) {   /* Send first notification */
-                               sip_send_mwi_to_peer(p->relatedpeer, 0);
+                               struct sip_peer *peer = p->relatedpeer;
+                               ref_peer(peer, "ensure a peer ref is held during MWI sending");
+                               ao2_unlock(p);
+                               sip_send_mwi_to_peer(peer, 0);
+                               ao2_lock(p);
+                               unref_peer(peer, "release a peer ref now that MWI is sent");
                        }
                } else if (p->subscribed != CALL_COMPLETION) {
                        if ((firststate = ast_extension_state(NULL, p->context, p->exten)) < 0) {
@@ -25132,36 +25139,49 @@ static int get_cached_mwi(struct sip_peer *peer, int *new, int *old)
        return in_cache;
 }
 
-/*! \brief Send message waiting indication to alert peer that they've got voicemail */
+/*! \brief Send message waiting indication to alert peer that they've got voicemail
+ *  \note Both peer and associated sip_pvt must be unlocked prior to calling this function
+*/
 static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
 {
        /* Called with peerl lock, but releases it */
        struct sip_pvt *p;
        int newmsgs = 0, oldmsgs = 0;
+       const char *vmexten;
+
+       ao2_lock(peer);
+
+       vmexten = ast_strdupa(peer->vmexten);
 
-       if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt)
+       if (ast_test_flag((&peer->flags[1]), SIP_PAGE2_SUBSCRIBEMWIONLY) && !peer->mwipvt) {
+               ao2_unlock(peer);
                return 0;
+       }
 
        /* Do we have an IP address? If not, skip this peer */
-       if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr))
+       if (ast_sockaddr_isnull(&peer->addr) && ast_sockaddr_isnull(&peer->defaddr)) {
+               ao2_unlock(peer);
                return 0;
+       }
 
        /* Attempt to use cached mwi to get message counts. */
        if (!get_cached_mwi(peer, &newmsgs, &oldmsgs) && !cache_only) {
                /* Fall back to manually checking the mailbox if not cache_only and get_cached_mwi failed */
                struct ast_str *mailbox_str = ast_str_alloca(512);
                peer_mailboxes_to_str(&mailbox_str, peer);
+               ao2_unlock(peer);
                ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs);
+               ao2_lock(peer);
        }
-       ao2_lock(peer);
 
        if (peer->mwipvt) {
                /* Base message on subscription */
-               p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt-- should this be done?");
+               p = dialog_ref(peer->mwipvt, "sip_send_mwi_to_peer: Setting dialog ptr p from peer->mwipvt");
+               ao2_unlock(peer);
        } else {
+               ao2_unlock(peer);
                /* Build temporary dialog for this message */
                if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) {
-                       ao2_unlock(peer);
                        return -1;
                }
 
@@ -25175,7 +25195,6 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
                        dialog_unlink_all(p);
                        dialog_unref(p, "unref dialog p just created via sip_alloc");
                        /* sip_destroy(p); */
-                       ao2_unlock(peer);
                        return 0;
                }
                /* Recalculate our side, and recalculate Call ID */
@@ -25183,11 +25202,15 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
                build_via(p);
                ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name");
                build_callid_pvt(p);
+
+               ao2_lock(peer);
                if (!ast_strlen_zero(peer->mwi_from)) {
                        ast_string_field_set(p, mwi_from, peer->mwi_from);
                } else if (!ast_strlen_zero(default_mwi_from)) {
                        ast_string_field_set(p, mwi_from, default_mwi_from);
                }
+               ao2_unlock(peer);
+
                ao2_t_link(dialogs, p, "Linking in under new name");
                /* Destroy this session after 32 secs */
                sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
@@ -25200,10 +25223,10 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
        /* Send MWI */
        ast_set_flag(&p->flags[0], SIP_OUTGOING);
        /* the following will decrement the refcount on p as it finishes */
-       transmit_notify_with_mwi(p, newmsgs, oldmsgs, peer->vmexten);
+       transmit_notify_with_mwi(p, newmsgs, oldmsgs, vmexten);
        sip_pvt_unlock(p);
        dialog_unref(p, "unref dialog ptr p just before it goes out of scope at the end of sip_send_mwi_to_peer.");
-       ao2_unlock(peer);
+
        return 0;
 }