From: Mark Michelson Date: Mon, 15 Oct 2012 21:06:42 +0000 (+0000) Subject: Fix some potential misuses of ast_str in the code. X-Git-Tag: 10.11.0-rc1~36 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=3e2a2ac24fea52b2bb6f80e906d45e6ae6f645ff;p=thirdparty%2Fasterisk.git Fix some potential misuses of ast_str in the code. Passing an ast_str pointer by value that then calls ast_str_set(), ast_str_set_va(), ast_str_append(), or ast_str_append_va() can result in the pointer originally passed by value being invalidated if the ast_str had to be reallocated. This fixes places in the code that do this. Only the example in ccss.c could result in pointer invalidation though since the other cases use a stack-allocated ast_str and cannot be reallocated. I've also updated the doxygen in strings.h to include notes about potential misuse of the functions mentioned previously. Review: https://reviewboard.asterisk.org/r/2161 ........ Merged revisions 375025 from http://svn.asterisk.org/svn/asterisk/branches/1.8 git-svn-id: https://origsvn.digium.com/svn/asterisk/branches/10@375026 65c4cc65-6c06-0410-ace0-fbb531ad65f3 --- diff --git a/apps/app_dial.c b/apps/app_dial.c index c3573f907d..247b4466a8 100644 --- a/apps/app_dial.c +++ b/apps/app_dial.c @@ -672,7 +672,7 @@ struct chanlist { struct ast_aoc_decoded *aoc_s_rate_list; }; -static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str *featurecode); +static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str **featurecode); static void chanlist_free(struct chanlist *outgoing) { @@ -1532,7 +1532,7 @@ static struct ast_channel *wait_for_answer(struct ast_channel *in, } if (ast_test_flag64(peerflags, OPT_CALLER_HANGUP) && - detect_disconnect(in, f->subclass.integer, featurecode)) { + detect_disconnect(in, f->subclass.integer, &featurecode)) { ast_verb(3, "User requested call disconnect.\n"); *to = 0; strcpy(pa->status, "CANCEL"); @@ -1645,18 +1645,18 @@ skip_frame:; return peer; } -static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str *featurecode) +static int detect_disconnect(struct ast_channel *chan, char code, struct ast_str **featurecode) { struct ast_flags features = { AST_FEATURE_DISCONNECT }; /* only concerned with disconnect feature */ struct ast_call_feature feature = { 0, }; int res; - ast_str_append(&featurecode, 1, "%c", code); + ast_str_append(featurecode, 1, "%c", code); - res = ast_feature_detect(chan, &features, ast_str_buffer(featurecode), &feature); + res = ast_feature_detect(chan, &features, ast_str_buffer(*featurecode), &feature); if (res != AST_FEATURE_RETURN_STOREDIGITS) { - ast_str_reset(featurecode); + ast_str_reset(*featurecode); } if (feature.feature_mask & AST_FEATURE_DISCONNECT) { return 1; diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c index a446d60f90..a0d32d68a1 100644 --- a/channels/chan_iax2.c +++ b/channels/chan_iax2.c @@ -1537,19 +1537,19 @@ static int send_ping(const void *data) return 0; } -static void encmethods_to_str(int e, struct ast_str *buf) +static void encmethods_to_str(int e, struct ast_str **buf) { - ast_str_set(&buf, 0, "("); + ast_str_set(buf, 0, "("); if (e & IAX_ENCRYPT_AES128) { - ast_str_append(&buf, 0, "aes128"); + ast_str_append(buf, 0, "aes128"); } if (e & IAX_ENCRYPT_KEYROTATE) { - ast_str_append(&buf, 0, ",keyrotate"); + ast_str_append(buf, 0, ",keyrotate"); } - if (ast_str_strlen(buf) > 1) { - ast_str_append(&buf, 0, ")"); + if (ast_str_strlen(*buf) > 1) { + ast_str_append(buf, 0, ")"); } else { - ast_str_set(&buf, 0, "No"); + ast_str_set(buf, 0, "No"); } } @@ -3855,7 +3855,7 @@ static char *handle_cli_iax2_show_peer(struct ast_cli_entry *e, int cmd, struct ast_sockaddr_to_sin(&peer->addr, &peer_addr); - encmethods_to_str(peer->encmethods, encmethods); + encmethods_to_str(peer->encmethods, &encmethods); ast_cli(a->fd, "\n\n"); ast_cli(a->fd, " * Name : %s\n", peer->name); ast_cli(a->fd, " Description : %s\n", peer->description); @@ -6822,7 +6822,7 @@ static int __iax2_show_peers(int fd, int *total, struct mansession *s, const int else ast_copy_string(name, peer->name, sizeof(name)); - encmethods_to_str(peer->encmethods, encmethods); + encmethods_to_str(peer->encmethods, &encmethods); retstatus = peer_status(peer, status, sizeof(status)); if (retstatus > 0) online_peers++; @@ -7130,7 +7130,7 @@ static int manager_iax2_show_peer_list(struct mansession *s, const struct messag i = ao2_iterator_init(peers, 0); for (; (peer = ao2_iterator_next(&i)); peer_unref(peer)) { - encmethods_to_str(peer->encmethods, encmethods); + encmethods_to_str(peer->encmethods, &encmethods); astman_append(s, "Event: PeerEntry\r\n%sChanneltype: IAX\r\n", idtext); if (!ast_strlen_zero(peer->username)) { astman_append(s, "ObjectName: %s\r\nObjectUsername: %s\r\n", peer->name, peer->username); @@ -14716,7 +14716,7 @@ static int peers_data_provider_get(const struct ast_data_search *search, ast_data_add_bool(data_peer, "dynamic", ast_test_flag64(peer, IAX_DYNAMIC)); - encmethods_to_str(peer->encmethods, encmethods); + encmethods_to_str(peer->encmethods, &encmethods); ast_data_add_str(data_peer, "encryption", peer->encmethods ? ast_str_buffer(encmethods) : "no"); peer_unref(peer); diff --git a/include/asterisk/strings.h b/include/asterisk/strings.h index 40477da7f0..cb61788da8 100644 --- a/include/asterisk/strings.h +++ b/include/asterisk/strings.h @@ -778,6 +778,12 @@ char *__ast_str_helper2(struct ast_str **buf, ssize_t max_len, * ... * } * \endcode + * + * \note Care should be taken when using this function. The function can + * result in reallocating the ast_str. If a pointer to the ast_str is passed + * by value to a function that calls ast_str_set_va(), then the original ast_str + * pointer may be invalidated due to a reallocation. + * */ AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_set_va(struct ast_str **buf, ssize_t max_len, const char *fmt, va_list ap), { @@ -789,6 +795,12 @@ AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_set_va(struct a * \brief Append to a dynamic string using a va_list * * Same as ast_str_set_va(), but append to the current content. + * + * \note Care should be taken when using this function. The function can + * result in reallocating the ast_str. If a pointer to the ast_str is passed + * by value to a function that calls ast_str_append_va(), then the original ast_str + * pointer may be invalidated due to a reallocation. + * */ AST_INLINE_API(int __attribute__((format(printf, 3, 0))) ast_str_append_va(struct ast_str **buf, ssize_t max_len, const char *fmt, va_list ap), { @@ -827,6 +839,11 @@ AST_INLINE_API(char *ast_str_append_escapecommas(struct ast_str **buf, ssize_t m /*! * \brief Set a dynamic string using variable arguments * + * \note Care should be taken when using this function. The function can + * result in reallocating the ast_str. If a pointer to the ast_str is passed + * by value to a function that calls ast_str_set(), then the original ast_str + * pointer may be invalidated due to a reallocation. + * * \param buf This is the address of a pointer to a struct ast_str which should * have been retrieved using ast_str_thread_get. It will need to * be updated in the case that the buffer has to be reallocated to @@ -859,6 +876,11 @@ int __attribute__((format(printf, 3, 4))) ast_str_set( /*! * \brief Append to a thread local dynamic string * + * \note Care should be taken when using this function. The function can + * result in reallocating the ast_str. If a pointer to the ast_str is passed + * by value to a function that calls ast_str_append(), then the original ast_str + * pointer may be invalidated due to a reallocation. + * * The arguments, return values, and usage of this function are the same as * ast_str_set(), but the new data is appended to the current value. */ diff --git a/main/ccss.c b/main/ccss.c index 5e8639d6d1..c6aeae132f 100644 --- a/main/ccss.c +++ b/main/ccss.c @@ -3410,7 +3410,7 @@ struct ast_cc_monitor *ast_cc_get_monitor_by_recall_core_id(const int core_id, c * \param dialstring A new dialstring to add * \retval void */ -static void cc_unique_append(struct ast_str *str, const char *dialstring) +static void cc_unique_append(struct ast_str **str, const char *dialstring) { char dialstring_search[AST_CHANNEL_NAME]; @@ -3419,10 +3419,10 @@ static void cc_unique_append(struct ast_str *str, const char *dialstring) return; } snprintf(dialstring_search, sizeof(dialstring_search), "%s%c", dialstring, '&'); - if (strstr(ast_str_buffer(str), dialstring_search)) { + if (strstr(ast_str_buffer(*str), dialstring_search)) { return; } - ast_str_append(&str, 0, "%s", dialstring_search); + ast_str_append(str, 0, "%s", dialstring_search); } /*! @@ -3440,7 +3440,7 @@ static void cc_unique_append(struct ast_str *str, const char *dialstring) * \param str Where we will store CC_INTERFACES * \retval void */ -static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_str *str) +static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, struct ast_str **str) { struct extension_monitor_pvt *extension_pvt; struct extension_child_dialstring *child_dialstring; @@ -3449,7 +3449,7 @@ static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, s size_t length; /* Init to an empty string. */ - ast_str_truncate(str, 0); + ast_str_truncate(*str, 0); /* First we need to take all of the is_valid child_dialstrings from * the extension monitor we found and add them to the CC_INTERFACES @@ -3472,9 +3472,9 @@ static void build_cc_interfaces_chanvar(struct ast_cc_monitor *starting_point, s /* str will have an extra '&' tacked onto the end of it, so we need * to get rid of that. */ - length = ast_str_strlen(str); + length = ast_str_strlen(*str); if (length) { - ast_str_truncate(str, length - 1); + ast_str_truncate(*str, length - 1); } if (length <= 1) { /* Nothing to recall? This should not happen. */ @@ -3509,7 +3509,7 @@ int ast_cc_agent_set_interfaces_chanvar(struct ast_channel *chan) AST_LIST_LOCK(interface_tree); monitor = AST_LIST_FIRST(interface_tree); - build_cc_interfaces_chanvar(monitor, str); + build_cc_interfaces_chanvar(monitor, &str); AST_LIST_UNLOCK(interface_tree); pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str)); @@ -3561,7 +3561,7 @@ int ast_set_cc_interfaces_chanvar(struct ast_channel *chan, const char * const e return -1; } - build_cc_interfaces_chanvar(monitor_iter, str); + build_cc_interfaces_chanvar(monitor_iter, &str); AST_LIST_UNLOCK(interface_tree); pbx_builtin_setvar_helper(chan, "CC_INTERFACES", ast_str_buffer(str));