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/
........

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

Merged revisions 343635 from http://svn.asterisk.org/svn/asterisk/branches/10


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@343636 65c4cc65-6c06-0410-ace0-fbb531ad65f3
certified/11.2
Kinsey Moore 14 years ago
parent 00a522c000
commit 1c526d3d7d

@ -14855,7 +14855,9 @@ static enum check_auth_result register_verify(struct sip_pvt *p, struct ast_sock
} }
} }
if (!res) { if (!res) {
ao2_unlock(p);
sip_send_mwi_to_peer(peer, 0); sip_send_mwi_to_peer(peer, 0);
ao2_lock(p);
ast_devstate_changed(AST_DEVICE_UNKNOWN, "SIP/%s", peer->name); ast_devstate_changed(AST_DEVICE_UNKNOWN, "SIP/%s", peer->name);
} }
if (res < 0) { if (res < 0) {
@ -25023,7 +25025,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
p->subscribed = MWI_NOTIFICATION; p->subscribed = MWI_NOTIFICATION;
if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) { if (ast_test_flag(&authpeer->flags[1], SIP_PAGE2_SUBSCRIBEMWIONLY)) {
ao2_unlock(p);
add_peer_mwi_subs(authpeer); add_peer_mwi_subs(authpeer);
ao2_lock(p);
} }
if (authpeer->mwipvt && authpeer->mwipvt != p) { /* Destroy old PVT if this is a new one */ if (authpeer->mwipvt && authpeer->mwipvt != p) { /* Destroy old PVT if this is a new one */
/* We only allow one subscription per peer */ /* We only allow one subscription per peer */
@ -25099,7 +25103,12 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
transmit_response(p, "200 OK", req); transmit_response(p, "200 OK", req);
if (p->relatedpeer) { /* Send first notification */ if (p->relatedpeer) { /* Send first notification */
sip_send_mwi_to_peer(p->relatedpeer, 0); struct sip_peer *peer = p->relatedpeer;
sip_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);
sip_unref_peer(peer, "release a peer ref now that MWI is sent");
} }
} else if (p->subscribed != CALL_COMPLETION) { } else if (p->subscribed != CALL_COMPLETION) {
@ -25835,6 +25844,7 @@ static int get_cached_mwi(struct sip_peer *peer, int *new, int *old)
} }
/*! \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
* \returns -1 on failure, 0 on success * \returns -1 on failure, 0 on success
*/ */
static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
@ -25842,13 +25852,20 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
/* Called with peerl lock, but releases it */ /* Called with peerl lock, but releases it */
struct sip_pvt *p; struct sip_pvt *p;
int newmsgs = 0, oldmsgs = 0; 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 -1; return -1;
} }
/* Do we have an IP address? If not, skip this peer */ /* 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 -1; return -1;
} }
@ -25861,17 +25878,19 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
if (ast_strlen_zero(mailbox_str->str)) { if (ast_strlen_zero(mailbox_str->str)) {
return -1; return -1;
} }
ao2_unlock(peer);
ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs); ast_app_inboxcount(mailbox_str->str, &newmsgs, &oldmsgs);
ao2_lock(peer);
} }
ao2_lock(peer);
if (peer->mwipvt) { if (peer->mwipvt) {
/* Base message on subscription */ /* 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 { } else {
ao2_unlock(peer);
/* Build temporary dialog for this message */ /* Build temporary dialog for this message */
if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) { if (!(p = sip_alloc(NULL, NULL, 0, SIP_NOTIFY, NULL))) {
ao2_unlock(peer);
return -1; return -1;
} }
@ -25885,7 +25904,6 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
dialog_unlink_all(p); dialog_unlink_all(p);
dialog_unref(p, "unref dialog p just created via sip_alloc"); dialog_unref(p, "unref dialog p just created via sip_alloc");
/* sip_destroy(p); */ /* sip_destroy(p); */
ao2_unlock(peer);
return -1; return -1;
} }
/* Recalculate our side, and recalculate Call ID */ /* Recalculate our side, and recalculate Call ID */
@ -25893,11 +25911,15 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
build_via(p); build_via(p);
ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name"); ao2_t_unlink(dialogs, p, "About to change the callid -- remove the old name");
build_callid_pvt(p); build_callid_pvt(p);
ao2_lock(peer);
if (!ast_strlen_zero(peer->mwi_from)) { if (!ast_strlen_zero(peer->mwi_from)) {
ast_string_field_set(p, mwi_from, peer->mwi_from); ast_string_field_set(p, mwi_from, peer->mwi_from);
} else if (!ast_strlen_zero(default_mwi_from)) { } else if (!ast_strlen_zero(default_mwi_from)) {
ast_string_field_set(p, mwi_from, 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"); ao2_t_link(dialogs, p, "Linking in under new name");
/* Destroy this session after 32 secs */ /* Destroy this session after 32 secs */
sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT); sip_scheddestroy(p, DEFAULT_TRANS_TIMEOUT);
@ -25910,10 +25932,10 @@ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only)
/* Send MWI */ /* Send MWI */
ast_set_flag(&p->flags[0], SIP_OUTGOING); ast_set_flag(&p->flags[0], SIP_OUTGOING);
/* the following will decrement the refcount on p as it finishes */ /* 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); 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."); 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; return 0;
} }

Loading…
Cancel
Save