Fix a crash that would occur when receiving a 491 response to a reinvite.

The reviewboard description does a fine job of summarizing this, so here it is:

A reporter discovered that Asterisk would crash when attempting to retransmit
a reinvite that had previously received a 491 response. The crash occurred
because a pjsip_tx_data structure was being saved for reuse, but its reference
count was not being increased. The result was that the pjsip_tx_data was being
freed before we were actually done with it. When we attempted to re-use the
structure when re-sending the reinvite, Asterisk would crash.

The fix implemented here is not to try holding onto the pjsip_tx_data at all.
Instead, when we reschedule sending the reinvite, we create a brand new
pjsip_tx_data and send that instead. Because of this change, there is no need
for an ast_sip_session_delayed_request structure to have a pjsip_tx_data on
it any more. So any code referencing its use has been removed.

When this initial fix was introduced, I encountered a second crash when
processing a subsequent 200 OK on a rescheduled reinvite. The reason was
that when rescheduling the reinvite, we gave the wrong location for a
response callback. This has been fixed in this patch as well.

ASTERISK-24556 #close
Reported by Abhay Gupta

Review: https://reviewboard.asterisk.org/r/4233
........

Merged revisions 429089 from http://svn.asterisk.org/svn/asterisk/branches/13


git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@429090 65c4cc65-6c06-0410-ace0-fbb531ad65f3
This commit is contained in:
Mark Michelson
2014-12-08 16:43:00 +00:00
parent fe7671fee6
commit bba1763f47

View File

@@ -497,8 +497,6 @@ struct ast_sip_session_delayed_request {
ast_sip_session_response_cb on_response; ast_sip_session_response_cb on_response;
/*! Whether to generate new SDP */ /*! Whether to generate new SDP */
int generate_new_sdp; int generate_new_sdp;
/*! Request to send */
pjsip_tx_data *tdata;
AST_LIST_ENTRY(ast_sip_session_delayed_request) next; AST_LIST_ENTRY(ast_sip_session_delayed_request) next;
}; };
@@ -506,8 +504,7 @@ static struct ast_sip_session_delayed_request *delayed_request_alloc(const char
ast_sip_session_request_creation_cb on_request_creation, ast_sip_session_request_creation_cb on_request_creation,
ast_sip_session_sdp_creation_cb on_sdp_creation, ast_sip_session_sdp_creation_cb on_sdp_creation,
ast_sip_session_response_cb on_response, ast_sip_session_response_cb on_response,
int generate_new_sdp, int generate_new_sdp)
pjsip_tx_data *tdata)
{ {
struct ast_sip_session_delayed_request *delay = ast_calloc(1, sizeof(*delay)); struct ast_sip_session_delayed_request *delay = ast_calloc(1, sizeof(*delay));
if (!delay) { if (!delay) {
@@ -518,7 +515,6 @@ static struct ast_sip_session_delayed_request *delayed_request_alloc(const char
delay->on_sdp_creation = on_sdp_creation; delay->on_sdp_creation = on_sdp_creation;
delay->on_response = on_response; delay->on_response = on_response;
delay->generate_new_sdp = generate_new_sdp; delay->generate_new_sdp = generate_new_sdp;
delay->tdata = tdata;
return delay; return delay;
} }
@@ -526,11 +522,6 @@ static int send_delayed_request(struct ast_sip_session *session, struct ast_sip_
{ {
ast_debug(3, "Sending delayed %s request to %s\n", delay->method, ast_sorcery_object_get_id(session->endpoint)); ast_debug(3, "Sending delayed %s request to %s\n", delay->method, ast_sorcery_object_get_id(session->endpoint));
if (delay->tdata) {
ast_sip_session_send_request_with_cb(session, delay->tdata, delay->on_response);
return 0;
}
if (!strcmp(delay->method, "INVITE")) { if (!strcmp(delay->method, "INVITE")) {
ast_sip_session_refresh(session, delay->on_request_creation, ast_sip_session_refresh(session, delay->on_request_creation,
delay->on_sdp_creation, delay->on_response, AST_SIP_SESSION_REFRESH_METHOD_INVITE, delay->generate_new_sdp); delay->on_sdp_creation, delay->on_response, AST_SIP_SESSION_REFRESH_METHOD_INVITE, delay->generate_new_sdp);
@@ -573,10 +564,10 @@ static void queue_delayed_request(struct ast_sip_session *session)
static int delay_request(struct ast_sip_session *session, ast_sip_session_request_creation_cb on_request, static int delay_request(struct ast_sip_session *session, ast_sip_session_request_creation_cb on_request,
ast_sip_session_sdp_creation_cb on_sdp_creation, ast_sip_session_response_cb on_response, ast_sip_session_sdp_creation_cb on_sdp_creation, ast_sip_session_response_cb on_response,
int generate_new_sdp, const char *method, pjsip_tx_data *tdata) int generate_new_sdp, const char *method)
{ {
struct ast_sip_session_delayed_request *delay = delayed_request_alloc(method, struct ast_sip_session_delayed_request *delay = delayed_request_alloc(method,
on_request, on_sdp_creation, on_response, generate_new_sdp, tdata); on_request, on_sdp_creation, on_response, generate_new_sdp);
if (!delay) { if (!delay) {
return -1; return -1;
@@ -621,7 +612,7 @@ int ast_sip_session_refresh(struct ast_sip_session *session,
ast_debug(3, "Delaying sending request to %s because dialog has not been established...\n", ast_debug(3, "Delaying sending request to %s because dialog has not been established...\n",
ast_sorcery_object_get_id(session->endpoint)); ast_sorcery_object_get_id(session->endpoint));
return delay_request(session, on_request_creation, on_sdp_creation, on_response, generate_new_sdp, return delay_request(session, on_request_creation, on_sdp_creation, on_response, generate_new_sdp,
method == AST_SIP_SESSION_REFRESH_METHOD_INVITE ? "INVITE" : "UPDATE", NULL); method == AST_SIP_SESSION_REFRESH_METHOD_INVITE ? "INVITE" : "UPDATE");
} }
if (method == AST_SIP_SESSION_REFRESH_METHOD_INVITE) { if (method == AST_SIP_SESSION_REFRESH_METHOD_INVITE) {
@@ -630,7 +621,7 @@ int ast_sip_session_refresh(struct ast_sip_session *session,
ast_debug(3, "Delaying sending reinvite to %s because of outstanding transaction...\n", ast_debug(3, "Delaying sending reinvite to %s because of outstanding transaction...\n",
ast_sorcery_object_get_id(session->endpoint)); ast_sorcery_object_get_id(session->endpoint));
return delay_request(session, on_request_creation, on_sdp_creation, on_response, return delay_request(session, on_request_creation, on_sdp_creation, on_response,
generate_new_sdp, "INVITE", NULL); generate_new_sdp, "INVITE");
} else if (inv_session->state != PJSIP_INV_STATE_CONFIRMED) { } else if (inv_session->state != PJSIP_INV_STATE_CONFIRMED) {
/* Initial INVITE transaction failed to progress us to a confirmed state /* Initial INVITE transaction failed to progress us to a confirmed state
* which means re-invites are not possible * which means re-invites are not possible
@@ -647,7 +638,7 @@ int ast_sip_session_refresh(struct ast_sip_session *session,
ast_debug(3, "Delaying session refresh with new SDP to %s because SDP negotiation is not yet done...\n", ast_debug(3, "Delaying session refresh with new SDP to %s because SDP negotiation is not yet done...\n",
ast_sorcery_object_get_id(session->endpoint)); ast_sorcery_object_get_id(session->endpoint));
return delay_request(session, on_request_creation, on_sdp_creation, on_response, generate_new_sdp, return delay_request(session, on_request_creation, on_sdp_creation, on_response, generate_new_sdp,
method == AST_SIP_SESSION_REFRESH_METHOD_INVITE ? "INVITE" : "UPDATE", NULL); method == AST_SIP_SESSION_REFRESH_METHOD_INVITE ? "INVITE" : "UPDATE");
} }
new_sdp = generate_session_refresh_sdp(session); new_sdp = generate_session_refresh_sdp(session);
@@ -1728,10 +1719,10 @@ static void resend_reinvite(pj_timer_heap_t *timer, pj_timer_entry *entry)
ast_sip_push_task(rrd->session->serializer, really_resend_reinvite, entry->user_data); ast_sip_push_task(rrd->session->serializer, really_resend_reinvite, entry->user_data);
} }
static void reschedule_reinvite(struct ast_sip_session *session, ast_sip_session_response_cb on_response, pjsip_tx_data *tdata) static void reschedule_reinvite(struct ast_sip_session *session, ast_sip_session_response_cb on_response)
{ {
struct ast_sip_session_delayed_request *delay = delayed_request_alloc("INVITE", struct ast_sip_session_delayed_request *delay = delayed_request_alloc("INVITE",
NULL, NULL, on_response, 1, tdata); NULL, NULL, on_response, 1);
pjsip_inv_session *inv = session->inv_session; pjsip_inv_session *inv = session->inv_session;
struct reschedule_reinvite_data *rrd = reschedule_reinvite_data_alloc(session, delay); struct reschedule_reinvite_data *rrd = reschedule_reinvite_data_alloc(session, delay);
pj_time_val tv; pj_time_val tv;
@@ -1981,6 +1972,7 @@ static void session_inv_on_tsx_state_changed(pjsip_inv_session *inv, pjsip_trans
tsx->mod_data[session_module.id] = e->body.tsx_state.src.tdata->mod_data[session_module.id]; tsx->mod_data[session_module.id] = e->body.tsx_state.src.tdata->mod_data[session_module.id];
break; break;
case PJSIP_EVENT_RX_MSG: case PJSIP_EVENT_RX_MSG:
cb = ast_sip_mod_data_get(tsx->mod_data, session_module.id, MOD_DATA_ON_RESPONSE);
handle_incoming(session, e->body.tsx_state.src.rdata, e->type, handle_incoming(session, e->body.tsx_state.src.rdata, e->type,
AST_SIP_SESSION_AFTER_MEDIA); AST_SIP_SESSION_AFTER_MEDIA);
if (tsx->method.id == PJSIP_INVITE_METHOD) { if (tsx->method.id == PJSIP_INVITE_METHOD) {
@@ -1988,7 +1980,7 @@ static void session_inv_on_tsx_state_changed(pjsip_inv_session *inv, pjsip_trans
if (tsx->state == PJSIP_TSX_STATE_COMPLETED) { if (tsx->state == PJSIP_TSX_STATE_COMPLETED) {
/* This means we got a non 2XX final response to our outgoing INVITE */ /* This means we got a non 2XX final response to our outgoing INVITE */
if (tsx->status_code == PJSIP_SC_REQUEST_PENDING) { if (tsx->status_code == PJSIP_SC_REQUEST_PENDING) {
reschedule_reinvite(session, tsx->mod_data[session_module.id], tsx->last_tx); reschedule_reinvite(session, cb);
return; return;
} else if (inv->state == PJSIP_INV_STATE_CONFIRMED && } else if (inv->state == PJSIP_INV_STATE_CONFIRMED &&
tsx->status_code != 488) { tsx->status_code != 488) {
@@ -2015,8 +2007,7 @@ static void session_inv_on_tsx_state_changed(pjsip_inv_session *inv, pjsip_trans
} }
} }
} }
if ((cb = ast_sip_mod_data_get(tsx->mod_data, session_module.id, if (cb) {
MOD_DATA_ON_RESPONSE))) {
cb(session, e->body.tsx_state.src.rdata); cb(session, e->body.tsx_state.src.rdata);
} }
case PJSIP_EVENT_TRANSPORT_ERROR: case PJSIP_EVENT_TRANSPORT_ERROR: