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

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

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


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@347069 65c4cc65-6c06-0410-ace0-fbb531ad65f3
certified/11.2
Matthew Jordan 14 years ago
parent 938b642245
commit b8dba87a4c

@ -24940,11 +24940,21 @@ static int handle_request_publish(struct sip_pvt *p, struct sip_request *req, st
return handler_result; 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) static void add_peer_mwi_subs(struct sip_peer *peer)
{ {
struct sip_mailbox *mailbox; struct sip_mailbox *mailbox;
AST_LIST_TRAVERSE(&peer->mailboxes, mailbox, entry) { 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, 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_MAILBOX, AST_EVENT_IE_PLTYPE_STR, mailbox->mailbox,
AST_EVENT_IE_CONTEXT, AST_EVENT_IE_PLTYPE_STR, S_OR(mailbox->context, "default"), AST_EVENT_IE_CONTEXT, AST_EVENT_IE_PLTYPE_STR, S_OR(mailbox->context, "default"),
@ -25098,7 +25108,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 an authentication response was sent, we are done here */
if (res == AUTH_CHALLENGE_SENT) /* authpeer = NULL here */ if (res == AUTH_CHALLENGE_SENT) /* authpeer = NULL here */
return 0; return 0;
if (res < 0) { if (res != AUTH_SUCCESSFUL) {
if (res == AUTH_FAKE_AUTH) { if (res == AUTH_FAKE_AUTH) {
ast_log(LOG_NOTICE, "Sending fake auth rejection for device %s\n", sip_get_header(req, "From")); ast_log(LOG_NOTICE, "Sending fake auth rejection for device %s\n", sip_get_header(req, "From"));
transmit_fake_auth_response(p, SIP_SUBSCRIBE, req, XMIT_UNRELIABLE); transmit_fake_auth_response(p, SIP_SUBSCRIBE, req, XMIT_UNRELIABLE);
@ -25112,17 +25122,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, /* At this point, we hold a reference to authpeer (if not NULL). It
* so we must release it when done. * must be released when done.
* XXX must remove all the checks for authpeer == NULL.
*/ */
/* Check if this device is allowed to subscribe at all */ /* Check if this device is allowed to subscribe at all */
if (!ast_test_flag(&p->flags[1], SIP_PAGE2_ALLOWSUBSCRIBE)) { if (!ast_test_flag(&p->flags[1], SIP_PAGE2_ALLOWSUBSCRIBE)) {
transmit_response(p, "403 Forbidden (policy)", req); transmit_response(p, "403 Forbidden (policy)", req);
pvt_set_needdestroy(p, "subscription not allowed"); pvt_set_needdestroy(p, "subscription not allowed");
if (authpeer) if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 1)"); sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 1)");
}
return 0; return 0;
} }
@ -25142,8 +25152,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
transmit_response(p, "404 Not Found", req); transmit_response(p, "404 Not Found", req);
} }
pvt_set_needdestroy(p, "subscription target not found"); pvt_set_needdestroy(p, "subscription target not found");
if (authpeer) if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 2)"); sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 2)");
}
return 0; return 0;
} }
@ -25158,9 +25169,6 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
enum subscriptiontype subscribed = NONE; enum subscriptiontype subscribed = NONE;
const char *unknown_acceptheader = NULL; const char *unknown_acceptheader = NULL;
if (authpeer) /* We do not need the authpeer any more */
authpeer = sip_unref_peer(authpeer, "sip_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 */ /* Header from Xten Eye-beam Accept: multipart/related, application/rlmi+xml, application/pidf+xml, application/xpidf+xml */
accept = __get_header(req, "Accept", &start); accept = __get_header(req, "Accept", &start);
while ((subscribed == NONE) && !ast_strlen_zero(accept)) { while ((subscribed == NONE) && !ast_strlen_zero(accept)) {
@ -25198,6 +25206,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
p->subscribecontext, p->subscribecontext,
p->subscribeuri); p->subscribeuri);
pvt_set_needdestroy(p, "no Accept header"); pvt_set_needdestroy(p, "no Accept header");
if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 2)");
}
return 0; return 0;
} }
/* if p->subscribed is non-zero, then accept is not obligatory; according to rfc 3265 section 3.1.3, at least. /* if p->subscribed is non-zero, then accept is not obligatory; according to rfc 3265 section 3.1.3, at least.
@ -25222,6 +25233,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
p->subscribecontext, p->subscribecontext,
p->subscribeuri); p->subscribeuri);
pvt_set_needdestroy(p, "unrecognized format"); pvt_set_needdestroy(p, "unrecognized format");
if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 2)");
}
return 0; return 0;
} else { } else {
p->subscribed = subscribed; p->subscribed = subscribed;
@ -25244,8 +25258,9 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
transmit_response(p, "406 Not Acceptable", req); transmit_response(p, "406 Not Acceptable", req);
ast_debug(2, "Received SIP mailbox subscription for unknown format: %s\n", acceptheader); ast_debug(2, "Received SIP mailbox subscription for unknown format: %s\n", acceptheader);
pvt_set_needdestroy(p, "unknown format"); pvt_set_needdestroy(p, "unknown format");
if (authpeer) if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 3)"); sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 3)");
}
return 0; return 0;
} }
/* Looks like they actually want a mailbox status /* Looks like they actually want a mailbox status
@ -25254,11 +25269,17 @@ 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 In most devices, this is configurable to the voicemailmain extension you use
*/ */
if (!authpeer || AST_LIST_EMPTY(&authpeer->mailboxes)) { 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"); 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) if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 4)"); sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 3)");
}
return 0; return 0;
} }
@ -25268,18 +25289,21 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
add_peer_mwi_subs(authpeer); add_peer_mwi_subs(authpeer);
ao2_lock(p); 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 */ /* We only allow one subscription per peer */
dialog_unlink_all(authpeer->mwipvt); if (authpeer->mwipvt) {
authpeer->mwipvt = dialog_unref(authpeer->mwipvt, "unref dialog authpeer->mwipvt"); dialog_unlink_all(authpeer->mwipvt);
/* sip_destroy(authpeer->mwipvt); */ authpeer->mwipvt = dialog_unref(authpeer->mwipvt, "unref dialog authpeer->mwipvt");
} }
if (authpeer->mwipvt) authpeer->mwipvt = dialog_ref(p, "setting peers' mwipvt to p");
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) if (p->relatedpeer != authpeer) {
sip_unref_peer(p->relatedpeer, "Unref previously stored relatedpeer ptr"); if (p->relatedpeer) {
p->relatedpeer = sip_ref_peer(authpeer, "setting dialog's relatedpeer pointer"); /* already refcounted...Link from pvt to peer UH- should this be dialog_ref()? */ sip_unref_peer(p->relatedpeer, "Unref previously stored relatedpeer ptr");
}
p->relatedpeer = sip_ref_peer(authpeer, "setting dialog's relatedpeer pointer");
}
/* Do not release authpeer here */ /* Do not release authpeer here */
} else if (!strcmp(event, "call-completion")) { } else if (!strcmp(event, "call-completion")) {
handle_cc_subscribe(p, req); handle_cc_subscribe(p, req);
@ -25287,16 +25311,12 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
transmit_response(p, "489 Bad Event", req); transmit_response(p, "489 Bad Event", req);
ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", event); ast_debug(2, "Received SIP subscribe for unknown event package: %s\n", event);
pvt_set_needdestroy(p, "unknown event package"); pvt_set_needdestroy(p, "unknown event package");
if (authpeer) if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 5)"); sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 5)");
}
return 0; return 0;
} }
/* At this point, if we have an authpeer we should unref it. */
if (authpeer) {
authpeer = sip_unref_peer(authpeer, "unref pointer into (*authpeer)");
}
/* Add subscription for extension state from the PBX core */ /* Add subscription for extension state from the PBX core */
if (p->subscribed != MWI_NOTIFICATION && p->subscribed != CALL_COMPLETION && !resubscribe) { if (p->subscribed != MWI_NOTIFICATION && p->subscribed != CALL_COMPLETION && !resubscribe) {
if (p->stateid > -1) { if (p->stateid > -1) {
@ -25321,6 +25341,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", "with Expire header less that 'minexpire' limit. Received \"Expire: %d\" min is %d\n",
p->exten, p->context, p->expiry, min_expiry); p->exten, p->context, p->expiry, min_expiry);
pvt_set_needdestroy(p, "Expires is less that the min expires allowed."); pvt_set_needdestroy(p, "Expires is less that the min expires allowed.");
if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 6)");
}
return 0; return 0;
} }
@ -25356,6 +25379,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)); 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); transmit_response(p, "404 Not found", req);
pvt_set_needdestroy(p, "no extension for SUBSCRIBE"); pvt_set_needdestroy(p, "no extension for SUBSCRIBE");
if (authpeer) {
sip_unref_peer(authpeer, "sip_unref_peer, from handle_request_subscribe (authpeer 6)");
}
return 0; return 0;
} }
ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED); ast_set_flag(&p->flags[1], SIP_PAGE2_DIALOG_ESTABLISHED);
@ -25371,6 +25397,10 @@ static int handle_request_subscribe(struct sip_pvt *p, struct sip_request *req,
pvt_set_needdestroy(p, "forcing expiration"); pvt_set_needdestroy(p, "forcing expiration");
} }
} }
if (authpeer) {
sip_unref_peer(authpeer, "unref pointer into (*authpeer)");
}
return 1; return 1;
} }
@ -26088,7 +26118,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) 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; struct sip_pvt *p;
int newmsgs = 0, oldmsgs = 0; int newmsgs = 0, oldmsgs = 0;
const char *vmexten = NULL; const char *vmexten = NULL;

Loading…
Cancel
Save