Audit ast_pjsip_rdata_get_endpoint() usage for ref leaks.

Valgrind found some memory leaks associated with
ast_pjsip_rdata_get_endpoint().  The leaks would manifest when sending
responses to OPTIONS requests, processing MESSAGE requests, and
res_pjsip supplements implementing the incoming_request callback.

* Fix ast_pjsip_rdata_get_endpoint() endpoint ref leaks in
res/res_pjsip.c:supplement_on_rx_request(),
res/res_pjsip/pjsip_options.c:send_options_response(),
res/res_pjsip_messaging.c:rx_data_to_ast_msg(), and
res/res_pjsip_messaging.c:send_response().

* Eliminated RAII_VAR() use with ast_pjsip_rdata_get_endpoint() in
res/res_pjsip_nat.c:nat_on_rx_message().

* Fixed inconsistent but benign return value in
res/res_pjsip/pjsip_options.c:options_on_rx_request().

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

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

git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@433223 65c4cc65-6c06-0410-ace0-fbb531ad65f3

diff --git a/res/res_pjsip.c b/res/res_pjsip.c
index 4e0a797..1d6af27 100644 (file)
--- a/res/res_pjsip.c
+++ b/res/res_pjsip.c
@@ -3263,8 +3263,13 @@ static pj_bool_t supplement_on_rx_request(pjsip_rx_data *rdata)
 
        AST_RWLIST_RDLOCK(&supplements);
        AST_LIST_TRAVERSE(&supplements, supplement, next) {
-               if (supplement->incoming_request && does_method_match(&rdata->msg_info.msg->line.req.method.name, supplement->method)) {
-                       supplement->incoming_request(ast_pjsip_rdata_get_endpoint(rdata), rdata);
+               if (supplement->incoming_request
+                       && does_method_match(&rdata->msg_info.msg->line.req.method.name, supplement->method)) {
+                       struct ast_sip_endpoint *endpoint;
+
+                       endpoint = ast_pjsip_rdata_get_endpoint(rdata);
+                       supplement->incoming_request(endpoint, rdata);
+                       ao2_cleanup(endpoint);
                }
        }
        AST_RWLIST_UNLOCK(&supplements);
@@ -3280,7 +3285,8 @@ int ast_sip_send_response(pjsip_response_addr *res_addr, pjsip_tx_data *tdata, s
 
        AST_RWLIST_RDLOCK(&supplements);
        AST_LIST_TRAVERSE(&supplements, supplement, next) {
-               if (supplement->outgoing_response && does_method_match(&cseq->method.name, supplement->method)) {
+               if (supplement->outgoing_response
+                       && does_method_match(&cseq->method.name, supplement->method)) {
                        supplement->outgoing_response(sip_endpoint, contact, tdata);
                }
        }

diff --git a/res/res_pjsip/pjsip_options.c b/res/res_pjsip/pjsip_options.c
index 44f33f3..0b14bed 100644 (file)
--- a/res/res_pjsip/pjsip_options.c
+++ b/res/res_pjsip/pjsip_options.c
@@ -609,17 +609,20 @@ static pj_status_t send_options_response(pjsip_rx_data *rdata, int code)
        if (dlg && trans) {
                status = pjsip_dlg_send_response(dlg, trans, tdata);
        } else {
-               /* Get where to send request. */
-               if ((status = pjsip_get_response_addr(
-                            tdata->pool, rdata, &res_addr)) != PJ_SUCCESS) {
-                       ast_log(LOG_ERROR, "Unable to get response address (%d)\n",
-                               status);
+               struct ast_sip_endpoint *endpoint;
+
+               /* Get where to send response. */
+               status = pjsip_get_response_addr(tdata->pool, rdata, &res_addr);
+               if (status != PJ_SUCCESS) {
+                       ast_log(LOG_ERROR, "Unable to get response address (%d)\n", status);
 
                        pjsip_tx_data_dec_ref(tdata);
                        return status;
                }
-               status = ast_sip_send_response(&res_addr, tdata,
-                                                  ast_pjsip_rdata_get_endpoint(rdata));
+
+               endpoint = ast_pjsip_rdata_get_endpoint(rdata);
+               status = ast_sip_send_response(&res_addr, tdata, endpoint);
+               ao2_cleanup(endpoint);
        }
 
        if (status != PJ_SUCCESS) {
@@ -648,7 +651,7 @@ static pj_bool_t options_on_rx_request(pjsip_rx_data *rdata)
        ruri = rdata->msg_info.msg->line.req.uri;
        if (!PJSIP_URI_SCHEME_IS_SIP(ruri) && !PJSIP_URI_SCHEME_IS_SIPS(ruri)) {
                send_options_response(rdata, 416);
-               return -1;
+               return PJ_TRUE;
        }
 
        sip_ruri = pjsip_uri_get_uri(ruri);

diff --git a/res/res_pjsip_messaging.c b/res/res_pjsip_messaging.c
index 8130908..f3ae5e6 100644 (file)
--- a/res/res_pjsip_messaging.c
+++ b/res/res_pjsip_messaging.c
@@ -427,13 +427,13 @@ static char *sip_to_pjsip(char *buf, int size, int capacity)
  */
 static enum pjsip_status_code rx_data_to_ast_msg(pjsip_rx_data *rdata, struct ast_msg *msg)
 {
-       struct ast_sip_endpoint *endpt = ast_pjsip_rdata_get_endpoint(rdata);
+       RAII_VAR(struct ast_sip_endpoint *, endpt, NULL, ao2_cleanup);
        pjsip_uri *ruri = rdata->msg_info.msg->line.req.uri;
        pjsip_sip_uri *sip_ruri;
        pjsip_name_addr *name_addr;
        char buf[MAX_BODY_SIZE];
        const char *field;
-       const char *context = S_OR(endpt->message_context, endpt->context);
+       const char *context;
        char exten[AST_MAX_EXTENSION];
        int res = 0;
        int size;
@@ -445,6 +445,10 @@ static enum pjsip_status_code rx_data_to_ast_msg(pjsip_rx_data *rdata, struct as
        sip_ruri = pjsip_uri_get_uri(ruri);
        ast_copy_pj_str(exten, &sip_ruri->user, AST_MAX_EXTENSION);
 
+       endpt = ast_pjsip_rdata_get_endpoint(rdata);
+       ast_assert(endpt != NULL);
+
+       context = S_OR(endpt->message_context, endpt->context);
        res |= ast_msg_set_context(msg, "%s", context);
        res |= ast_msg_set_exten(msg, "%s", exten);
 
@@ -617,13 +621,18 @@ static pj_status_t send_response(pjsip_rx_data *rdata, enum pjsip_status_code co
        if (dlg && tsx) {
                status = pjsip_dlg_send_response(dlg, tsx, tdata);
        } else {
-               /* Get where to send request. */
+               struct ast_sip_endpoint *endpoint;
+
+               /* Get where to send response. */
                status = pjsip_get_response_addr(tdata->pool, rdata, &res_addr);
                if (status != PJ_SUCCESS) {
                        ast_log(LOG_ERROR, "Unable to get response address (%d)\n", status);
                        return status;
                }
-               status = ast_sip_send_response(&res_addr, tdata, ast_pjsip_rdata_get_endpoint(rdata));
+
+               endpoint = ast_pjsip_rdata_get_endpoint(rdata);
+               status = ast_sip_send_response(&res_addr, tdata, endpoint);
+               ao2_cleanup(endpoint);
        }
 
        if (status != PJ_SUCCESS) {

diff --git a/res/res_pjsip_nat.c b/res/res_pjsip_nat.c
index b71a84b..6e093ab 100644 (file)
--- a/res/res_pjsip_nat.c
+++ b/res/res_pjsip_nat.c
@@ -72,8 +72,13 @@ static pj_bool_t handle_rx_message(struct ast_sip_endpoint *endpoint, pjsip_rx_d
 
 static pj_bool_t nat_on_rx_message(pjsip_rx_data *rdata)
 {
-       RAII_VAR(struct ast_sip_endpoint *, endpoint, ast_pjsip_rdata_get_endpoint(rdata), ao2_cleanup);
-       return handle_rx_message(endpoint, rdata);
+       pj_bool_t res;
+       struct ast_sip_endpoint *endpoint;
+
+       endpoint = ast_pjsip_rdata_get_endpoint(rdata);
+       res = handle_rx_message(endpoint, rdata);
+       ao2_cleanup(endpoint);
+       return res;
 }
 
 /*! \brief Structure which contains information about a transport */