From: Matthew Jordan Date: Tue, 6 Dec 2011 17:05:05 +0000 (+0000) Subject: Fixed crash from orphaned MWI subscriptions in chan_sip X-Git-Tag: 1.8.9.0-rc1~42 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c0ead6cb853d66155338d9413459b100eeeb8f5;p=thirdparty%2Fasterisk.git Fixed crash from orphaned MWI subscriptions in chan_sip This patch resolves the issue where MWI subscriptions are orphaned by subsequent SIP SUBSCRIBE messages. When a peer is removed, either by pruning realtime SIP peers or by unloading / loading chan_sip, the MWI subscriptions that were orphaned would still be on the event engine list of valid subscriptions but have a pointer to a peer that no longer was valid. When an MWI event would occur, this would cause a seg fault. (closes issue ASTERISK-18663) Reported by: Ross Beer Tested by: Ross Beer, Matt Jordan Patches: blf_mwi_diff_12_06_11.txt uploaded by Matt Jordan (license 6283) Review: https://reviewboard.asterisk.org/r/1610/ git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/1.8@347058 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/channels/chan_sip.c b/channels/chan_sip.c index fc1d41f823..226458fcc1 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -24233,11 +24233,21 @@ static int handle_request_publish(struct sip_pvt *p, struct sip_request *req, st return handler_result; } +/*! \internal \brief Subscribe to MWI events for the specified peer + * \note The peer cannot be locked during this method. sip_send_mwi_peer will + * attempt to lock the peer after the event subscription lock is held; if the peer is locked during + * this method then we will attempt to lock the event subscription lock but after the peer, creating + * a locking inversion. + */ static void add_peer_mwi_subs(struct sip_peer *peer) { struct sip_mailbox *mailbox; AST_LIST_TRAVERSE(&peer->mailboxes, mailbox, entry) { + if (mailbox->event_sub) { + ast_event_unsubscribe(mailbox->event_sub); + } + mailbox->event_sub = ast_event_subscribe(AST_EVENT_MWI, mwi_event_cb, "SIP mbox event", peer, AST_EVENT_IE_MAILBOX, AST_EVENT_IE_PLTYPE_STR, mailbox->mailbox, AST_EVENT_IE_CONTEXT, AST_EVENT_IE_PLTYPE_STR, S_OR(mailbox->context, "default"), @@ -24391,7 +24401,7 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, /* if an authentication response was sent, we are done here */ if (res == AUTH_CHALLENGE_SENT) /* authpeer = NULL here */ return 0; - if (res < 0) { + if (res != AUTH_SUCCESSFUL) { if (res == AUTH_FAKE_AUTH) { ast_log(LOG_NOTICE, "Sending fake auth rejection for device %s\n", get_header(req, "From")); transmit_fake_auth_response(p, SIP_SUBSCRIBE, req, XMIT_UNRELIABLE); @@ -24405,17 +24415,17 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, } } - /* At this point, authpeer cannot be NULL. Remember we hold a reference, - * so we must release it when done. - * XXX must remove all the checks for authpeer == NULL. + /* At this point, we hold a reference to authpeer (if not NULL). It + * must be released when done. */ /* Check if this device is allowed to subscribe at all */ if (!ast_test_flag(&p->flags[1], SIP_PAGE2_ALLOWSUBSCRIBE)) { transmit_response(p, "403 Forbidden (policy)", req); pvt_set_needdestroy(p, "subscription not allowed"); - if (authpeer) + if (authpeer) { unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 1)"); + } return 0; } @@ -24435,8 +24445,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, transmit_response(p, "404 Not Found", req); } pvt_set_needdestroy(p, "subscription target not found"); - if (authpeer) + if (authpeer) { unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 2)"); + } return 0; } @@ -24451,9 +24462,6 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, enum subscriptiontype subscribed = NONE; const char *unknown_acceptheader = NULL; - if (authpeer) /* We do not need the authpeer any more */ - authpeer = unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 2)"); - /* Header from Xten Eye-beam Accept: multipart/related, application/rlmi+xml, application/pidf+xml, application/xpidf+xml */ accept = __get_header(req, "Accept", &start); while ((subscribed == NONE) && !ast_strlen_zero(accept)) { @@ -24491,6 +24499,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, p->subscribecontext, p->subscribeuri); pvt_set_needdestroy(p, "no Accept header"); + if (authpeer) { + unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 2)"); + } return 0; } /* if p->subscribed is non-zero, then accept is not obligatory; according to rfc 3265 section 3.1.3, at least. @@ -24515,6 +24526,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, p->subscribecontext, p->subscribeuri); pvt_set_needdestroy(p, "unrecognized format"); + if (authpeer) { + unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 2)"); + } return 0; } else { p->subscribed = subscribed; @@ -24537,8 +24551,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, transmit_response(p, "406 Not Acceptable", req); ast_debug(2, "Received SIP mailbox subscription for unknown format: %s\n", acceptheader); pvt_set_needdestroy(p, "unknown format"); - if (authpeer) + if (authpeer) { unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 3)"); + } return 0; } /* Looks like they actually want a mailbox status @@ -24547,11 +24562,16 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, In most devices, this is configurable to the voicemailmain extension you use */ if (!authpeer || AST_LIST_EMPTY(&authpeer->mailboxes)) { - transmit_response(p, "404 Not found (no mailbox)", req); + if (!authpeer) { + transmit_response(p, "404 Not found", req); + } else { + transmit_response(p, "404 Not found (no mailbox)", req); + ast_log(LOG_NOTICE, "Received SIP subscribe for peer without mailbox: %s\n", S_OR(authpeer->name, "")); + } pvt_set_needdestroy(p, "received 404 response"); - ast_log(LOG_NOTICE, "Received SIP subscribe for peer without mailbox: %s\n", S_OR(authpeer->name, "")); - if (authpeer) - unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 4)"); + if (authpeer) { + unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 3)"); + } return 0; } @@ -24561,18 +24581,20 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, 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 != p) { /* Destroy old PVT if this is a new one */ /* We only allow one subscription per peer */ - dialog_unlink_all(authpeer->mwipvt); - authpeer->mwipvt = dialog_unref(authpeer->mwipvt, "unref dialog authpeer->mwipvt"); - /* sip_destroy(authpeer->mwipvt); */ - } - if (authpeer->mwipvt) - dialog_unref(authpeer->mwipvt, "Unref previously stored mwipvt dialog pointer"); - authpeer->mwipvt = dialog_ref(p, "setting peers' mwipvt to p"); /* Link from peer to pvt UH- should this be dialog_ref()? */ - if (p->relatedpeer) - unref_peer(p->relatedpeer, "Unref previously stored relatedpeer ptr"); - p->relatedpeer = ref_peer(authpeer, "setting dialog's relatedpeer pointer"); /* already refcounted...Link from pvt to peer UH- should this be dialog_ref()? */ + if (authpeer->mwipvt) { + dialog_unlink_all(authpeer->mwipvt); + authpeer->mwipvt = dialog_unref(authpeer->mwipvt, "unref dialog authpeer->mwipvt"); + } + authpeer->mwipvt = dialog_ref(p, "setting peers' mwipvt to p"); + } + if (p->relatedpeer != authpeer) { + if (p->relatedpeer) { + unref_peer(p->relatedpeer, "Unref previously stored relatedpeer ptr"); + } + p->relatedpeer = ref_peer(authpeer, "setting dialog's relatedpeer pointer"); + } /* Do not release authpeer here */ } else if (!strcmp(event, "call-completion")) { handle_cc_subscribe(p, req); @@ -24580,16 +24602,12 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, transmit_response(p, "489 Bad Event", req); ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", event); pvt_set_needdestroy(p, "unknown event package"); - if (authpeer) + if (authpeer) { unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 5)"); + } return 0; } - /* At this point, if we have an authpeer we should unref it. */ - if (authpeer) { - authpeer = unref_peer(authpeer, "unref pointer into (*authpeer)"); - } - /* Add subscription for extension state from the PBX core */ if (p->subscribed != MWI_NOTIFICATION && p->subscribed != CALL_COMPLETION && !resubscribe) { if (p->stateid > -1) { @@ -24614,6 +24632,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, "with Expire header less that 'minexpire' limit. Received \"Expire: %d\" min is %d\n", p->exten, p->context, p->expiry, min_expiry); pvt_set_needdestroy(p, "Expires is less that the min expires allowed."); + if (authpeer) { + unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 6)"); + } return 0; } @@ -24648,6 +24669,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, ast_log(LOG_NOTICE, "Got SUBSCRIBE for extension %s@%s from %s, but there is no hint for that extension.\n", p->exten, p->context, ast_sockaddr_stringify(&p->sa)); transmit_response(p, "404 Not found", req); pvt_set_needdestroy(p, "no extension for SUBSCRIBE"); + if (authpeer) { + unref_peer(authpeer, "unref_peer, from handle_request_subscribe (authpeer 6)"); + } return 0; } ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); @@ -24663,6 +24687,10 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req, pvt_set_needdestroy(p, "forcing expiration"); } } + + if (authpeer) { + unref_peer(authpeer, "unref pointer into (*authpeer)"); + } return 1; } @@ -25356,7 +25384,7 @@ static int get_cached_mwi(struct sip_peer *peer, int *new, int *old) */ static int sip_send_mwi_to_peer(struct sip_peer *peer, int cache_only) { - /* Called with peerl lock, but releases it */ + /* Called with peer lock, but releases it */ struct sip_pvt *p; int newmsgs = 0, oldmsgs = 0; const char *vmexten = NULL; diff --git a/channels/sip/include/sip.h b/channels/sip/include/sip.h index 29e0f834dc..29c38b7445 100644 --- a/channels/sip/include/sip.h +++ b/channels/sip/include/sip.h @@ -474,7 +474,7 @@ enum check_auth_result { AUTH_PEER_NOT_DYNAMIC = -6, AUTH_ACL_FAILED = -7, AUTH_BAD_TRANSPORT = -8, - AUTH_RTP_FAILED = 9, + AUTH_RTP_FAILED = -9, }; /*! \brief States for outbound registrations (with register= lines in sip.conf */