From 20750e261b3cc1cd954ac3de839afadcc4817d8d Mon Sep 17 00:00:00 2001 From: Richard Mudgett Date: Wed, 30 Apr 2014 21:03:29 +0000 Subject: [PATCH] chan_sip.c: Fixed off-nominal message iterator ref count and alloc fail issues. * Fixed early exit in sip_msg_send() not destroying the message iterator. * Made ast_msg_var_iterator_next() and ast_msg_var_iterator_destroy() tolerant of a NULL iter parameter in case ast_msg_var_iterator_init() fails. * Made ast_msg_var_iterator_destroy() clean up any current message data ref. * Made struct ast_msg_var_iterator, ast_msg_var_iterator_init(), ast_msg_var_iterator_next(), ast_msg_var_unref_current(), and ast_msg_var_iterator_destroy() use iter instead of i. * Eliminated RAII_VAR usage in res_pjsip_messaging.c:vars_to_headers(). ........ Merged revisions 413139 from http://svn.asterisk.org/svn/asterisk/branches/11 ........ Merged revisions 413142 from http://svn.asterisk.org/svn/asterisk/branches/12 git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@413144 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- channels/chan_sip.c | 8 ++++---- include/asterisk/message.h | 10 +++++----- main/message.c | 40 +++++++++++++++++++++++--------------- res/res_pjsip_messaging.c | 12 ++++++++---- 4 files changed, 41 insertions(+), 29 deletions(-) diff --git a/channels/chan_sip.c b/channels/chan_sip.c index 694ab74788..2ff1c50108 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -26715,12 +26715,11 @@ static int sip_msg_send(const struct ast_msg *msg, const char *to, const char *f return -1; } - for (iter = ast_msg_var_iterator_init(msg); - ast_msg_var_iterator_next(msg, iter, &var, &val); - ast_msg_var_unref_current(iter)) { + for (iter = ast_msg_var_iterator_init(msg); + ast_msg_var_iterator_next(msg, iter, &var, &val); + ast_msg_var_unref_current(iter)) { if (!strcasecmp(var, "Request-URI")) { ast_string_field_set(pvt, fullcontact, val); - ast_msg_var_unref_current(iter); break; } } @@ -26805,6 +26804,7 @@ static int sip_msg_send(const struct ast_msg *msg, const char *to, const char *f if (!strcasecmp(var, "Max-Forwards")) { /* Decrement Max-Forwards for SIP loop prevention. */ if (sscanf(val, "%30d", &pvt->maxforwards) != 1 || pvt->maxforwards < 1) { + ast_msg_var_iterator_destroy(iter); sip_pvt_unlock(pvt); dialog_unlink_all(pvt); dialog_unref(pvt, "MESSAGE(Max-Forwards) reached zero."); diff --git a/include/asterisk/message.h b/include/asterisk/message.h index d1f0bd722f..7e5c77a6a2 100644 --- a/include/asterisk/message.h +++ b/include/asterisk/message.h @@ -246,25 +246,25 @@ struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg /*! * \brief Get the next variable name and value that is set for sending outbound * \param msg The message with the variables - * \param i An iterator created with ast_msg_var_iterator_init + * \param iter An iterator created with ast_msg_var_iterator_init * \param name A pointer to the name result pointer * \param value A pointer to the value result pointer * * \retval 0 No more entries * \retval 1 Valid entry */ -int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *i, const char **name, const char **value); +int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value); /*! * \brief Destroy a message variable iterator - * \param i Iterator to be destroyed + * \param iter Iterator to be destroyed */ -void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *i); +void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *iter); /*! * \brief Unref a message var from inside an iterator loop */ -void ast_msg_var_unref_current(struct ast_msg_var_iterator *i); +void ast_msg_var_unref_current(struct ast_msg_var_iterator *iter); #if defined(__cplusplus) || defined(c_plusplus) } diff --git a/main/message.c b/main/message.c index bd706e3638..39166e0ff6 100644 --- a/main/message.c +++ b/main/message.c @@ -610,28 +610,34 @@ const char *ast_msg_get_var(struct ast_msg *msg, const char *name) } struct ast_msg_var_iterator { - struct ao2_iterator i; + struct ao2_iterator iter; struct msg_data *current_used; }; struct ast_msg_var_iterator *ast_msg_var_iterator_init(const struct ast_msg *msg) { - struct ast_msg_var_iterator *i; - if (!(i = ast_calloc(1, sizeof(*i)))) { + struct ast_msg_var_iterator *iter; + + iter = ast_calloc(1, sizeof(*iter)); + if (!iter) { return NULL; } - i->i = ao2_iterator_init(msg->vars, 0); + iter->iter = ao2_iterator_init(msg->vars, 0); - return i; + return iter; } -int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *i, const char **name, const char **value) +int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iterator *iter, const char **name, const char **value) { struct msg_data *data; + if (!iter) { + return 0; + } + /* Skip any that aren't marked for sending out */ - while ((data = ao2_iterator_next(&i->i)) && !data->send) { + while ((data = ao2_iterator_next(&iter->iter)) && !data->send) { ao2_ref(data, -1); } @@ -646,22 +652,24 @@ int ast_msg_var_iterator_next(const struct ast_msg *msg, struct ast_msg_var_iter /* Leave the refcount to be cleaned up by the caller with * ast_msg_var_unref_current after they finish with the pointers to the data */ - i->current_used = data; + iter->current_used = data; return 1; } -void ast_msg_var_unref_current(struct ast_msg_var_iterator *i) { - if (i->current_used) { - ao2_ref(i->current_used, -1); - } - i->current_used = NULL; +void ast_msg_var_unref_current(struct ast_msg_var_iterator *iter) +{ + ao2_cleanup(iter->current_used); + iter->current_used = NULL; } -void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *i) +void ast_msg_var_iterator_destroy(struct ast_msg_var_iterator *iter) { - ao2_iterator_destroy(&i->i); - ast_free(i); + if (iter) { + ao2_iterator_destroy(&iter->iter); + ast_msg_var_unref_current(iter); + ast_free(iter); + } } static struct ast_channel *create_msg_q_chan(void) diff --git a/res/res_pjsip_messaging.c b/res/res_pjsip_messaging.c index 07d207bdb2..86fc42788e 100644 --- a/res/res_pjsip_messaging.c +++ b/res/res_pjsip_messaging.c @@ -318,22 +318,26 @@ static enum pjsip_status_code vars_to_headers(const struct ast_msg *msg, pjsip_t const char *name; const char *value; int max_forwards; + struct ast_msg_var_iterator *iter; - RAII_VAR(struct ast_msg_var_iterator *, i, ast_msg_var_iterator_init(msg), ast_msg_var_iterator_destroy); - while (ast_msg_var_iterator_next(msg, i, &name, &value)) { + for (iter = ast_msg_var_iterator_init(msg); + ast_msg_var_iterator_next(msg, iter, &name, &value); + ast_msg_var_unref_current(iter)) { if (!strcasecmp(name, "Max-Forwards")) { /* Decrement Max-Forwards for SIP loop prevention. */ if (sscanf(value, "%30d", &max_forwards) != 1 || --max_forwards == 0) { + ast_msg_var_iterator_destroy(iter); ast_log(LOG_NOTICE, "MESSAGE(Max-Forwards) reached zero. MESSAGE not sent.\n"); return -1; } - sprintf((char*)value, "%d", max_forwards); + sprintf((char *) value, "%d", max_forwards); ast_sip_add_header(tdata, name, value); } else if (!is_msg_var_blocked(name)) { ast_sip_add_header(tdata, name, value); } - ast_msg_var_unref_current(i); } + ast_msg_var_iterator_destroy(iter); + return PJSIP_SC_OK; }