Author: mmichelson
Date: Mon Dec  8 10:41:45 2014
New Revision: 429089

URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=429089
Log:
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


Modified:
    branches/13/res/res_pjsip_session.c

Modified: branches/13/res/res_pjsip_session.c
URL: 
http://svnview.digium.com/svn/asterisk/branches/13/res/res_pjsip_session.c?view=diff&rev=429089&r1=429088&r2=429089
==============================================================================
--- branches/13/res/res_pjsip_session.c (original)
+++ branches/13/res/res_pjsip_session.c Mon Dec  8 10:41:45 2014
@@ -497,8 +497,6 @@
        ast_sip_session_response_cb on_response;
        /*! Whether to generate new SDP */
        int generate_new_sdp;
-       /*! Request to send */
-       pjsip_tx_data *tdata;
        AST_LIST_ENTRY(ast_sip_session_delayed_request) next;
 };
 
@@ -506,8 +504,7 @@
                ast_sip_session_request_creation_cb on_request_creation,
                ast_sip_session_sdp_creation_cb on_sdp_creation,
                ast_sip_session_response_cb on_response,
-               int generate_new_sdp,
-               pjsip_tx_data *tdata)
+               int generate_new_sdp)
 {
        struct ast_sip_session_delayed_request *delay = ast_calloc(1, 
sizeof(*delay));
        if (!delay) {
@@ -518,18 +515,12 @@
        delay->on_sdp_creation = on_sdp_creation;
        delay->on_response = on_response;
        delay->generate_new_sdp = generate_new_sdp;
-       delay->tdata = tdata;
        return delay;
 }
 
 static int send_delayed_request(struct ast_sip_session *session, struct 
ast_sip_session_delayed_request *delay)
 {
        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")) {
                ast_sip_session_refresh(session, delay->on_request_creation,
@@ -573,10 +564,10 @@
 
 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,
-               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,
-                       on_request, on_sdp_creation, on_response, 
generate_new_sdp, tdata);
+                       on_request, on_sdp_creation, on_response, 
generate_new_sdp);
 
        if (!delay) {
                return -1;
@@ -621,7 +612,7 @@
                ast_debug(3, "Delaying sending request to %s because dialog has 
not been established...\n",
                        ast_sorcery_object_get_id(session->endpoint));
                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) {
@@ -630,7 +621,7 @@
                        ast_debug(3, "Delaying sending reinvite to %s because 
of outstanding transaction...\n",
                                        
ast_sorcery_object_get_id(session->endpoint));
                        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) {
                        /* Initial INVITE transaction failed to progress us to 
a confirmed state
                         * which means re-invites are not possible
@@ -647,7 +638,7 @@
                        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));
                        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);
@@ -1728,10 +1719,10 @@
        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",
-                       NULL, NULL, on_response, 1, tdata);
+                       NULL, NULL, on_response, 1);
        pjsip_inv_session *inv = session->inv_session;
        struct reschedule_reinvite_data *rrd = 
reschedule_reinvite_data_alloc(session, delay);
        pj_time_val tv;
@@ -1981,6 +1972,7 @@
                tsx->mod_data[session_module.id] = 
e->body.tsx_state.src.tdata->mod_data[session_module.id];
                break;
        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,
                                AST_SIP_SESSION_AFTER_MEDIA);
                if (tsx->method.id == PJSIP_INVITE_METHOD) {
@@ -1988,7 +1980,7 @@
                                if (tsx->state == PJSIP_TSX_STATE_COMPLETED) {
                                        /* This means we got a non 2XX final 
response to our outgoing INVITE */
                                        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;
                                        } else if (inv->state == 
PJSIP_INV_STATE_CONFIRMED &&
                                                   tsx->status_code != 488) {
@@ -2015,8 +2007,7 @@
                                }
                        }
                }
-               if ((cb = ast_sip_mod_data_get(tsx->mod_data, session_module.id,
-                                              MOD_DATA_ON_RESPONSE))) {
+               if (cb) {
                        cb(session, e->body.tsx_state.src.rdata);
                }
        case PJSIP_EVENT_TRANSPORT_ERROR:


-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

svn-commits mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/svn-commits

Reply via email to