From: Richard Mudgett Date: Wed, 30 Apr 2014 20:26:16 +0000 (+0000) Subject: chan_sip.c: Fixed off-nominal message iterator ref count and alloc fail issues. X-Git-Tag: 11.10.0-rc1~30 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=709d39b66291bb3a88353648261db90335d161fb;p=thirdparty%2Fasterisk.git 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. git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/11@413139 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/channels/chan_sip.c b/channels/chan_sip.c index bbf7246fba..2125f068ca 100644 --- a/channels/chan_sip.c +++ b/channels/chan_sip.c @@ -26995,12 +26995,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; } } @@ -27085,6 +27084,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 31ed0b28a4..7e5c77a6a2 100644 --- a/include/asterisk/message.h +++ b/include/asterisk/message.h @@ -167,6 +167,7 @@ int __attribute__((format(printf, 2, 3))) * \brief Set a variable on the message going to the dialplan. * \note Setting a variable that already exists overwrites the existing variable value * + * \param msg * \param name Name of variable to set * \param value Value of variable to set * @@ -179,6 +180,7 @@ int ast_msg_set_var(struct ast_msg *msg, const char *name, const char *value); * \brief Set a variable on the message being sent to a message tech directly. * \note Setting a variable that already exists overwrites the existing variable value * + * \param msg * \param name Name of variable to set * \param value Value of variable to set * @@ -244,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 327d3b0c55..0dcf031970 100644 --- a/main/message.c +++ b/main/message.c @@ -606,28 +606,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); } @@ -642,22 +648,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) @@ -1321,10 +1329,14 @@ void ast_msg_shutdown(void) } } -/*! \internal \brief Clean up other resources on Asterisk shutdown +/*! + * \internal + * \brief Clean up other resources on Asterisk shutdown + * * \note This does not include the msg_q_tp object, which must be disposed * of prior to Asterisk checking for channel destruction in its shutdown - * sequence. The atexit handlers are executed after this occurs. */ + * sequence. The atexit handlers are executed after this occurs. + */ static void message_shutdown(void) { ast_custom_function_unregister(&msg_function);