diff options
author | Richard Mudgett <rmudgett@digium.com> | 2014-03-20 16:35:57 +0000 |
---|---|---|
committer | Richard Mudgett <rmudgett@digium.com> | 2014-03-20 16:35:57 +0000 |
commit | 1ba13718fc46527587e3dc87e776858c472bb2d2 (patch) | |
tree | ee6d210eb73de1a218310149733f99009456e402 | |
parent | 57239bfe37d61977ef6eec82eca0cb6631e1d4ef (diff) |
assigned-uniqueids: Miscellaneous cleanup and fixes.
* Fix memory leak in ast_unreal_new_channels(). Made it generate the ;2
uniqueid on a stack variable instead of mallocing it.
* Made send error response to ARI and AMI requests instead of just logging
excessive uniqueid length and allowing truncation. action_originate() and
ari_channels_handle_originate_with_id().
* Fixed minor truncating uniqueid hole when generating the ;2 uniqueid
string length. Created public and internal lengths of uniqueid. The
internal length can handle a max public uniqueid plus an appended ;2.
* free() and ast_free() are NULL tolerant so they don't need a NULL test
before calling.
* Made use better struct initialization format instead of the position
dependent initialization format. Also anything not explicitly initialized
in the struct is initialized to zero by the compiler.
* Made ast_channel_internal_set_fake_ids() use the safer
ast_copy_string() instead of strncpy().
Review: https://reviewboard.asterisk.org/r/3371/
........
Merged revisions 410949 from http://svn.asterisk.org/svn/asterisk/branches/12
git-svn-id: https://origsvn.digium.com/svn/asterisk/trunk@410950 65c4cc65-6c06-0410-ace0-fbb531ad65f3
-rw-r--r-- | include/asterisk/channel.h | 31 | ||||
-rw-r--r-- | include/asterisk/rtp_engine.h | 11 | ||||
-rw-r--r-- | main/channel_internal_api.c | 4 | ||||
-rw-r--r-- | main/core_unreal.c | 5 | ||||
-rw-r--r-- | main/dial.c | 28 | ||||
-rw-r--r-- | main/manager.c | 22 | ||||
-rw-r--r-- | res/ari/resource_channels.c | 17 | ||||
-rw-r--r-- | res/res_stasis_snoop.c | 4 |
8 files changed, 77 insertions, 45 deletions
diff --git a/include/asterisk/channel.h b/include/asterisk/channel.h index aa2f8634b..497ef0bd3 100644 --- a/include/asterisk/channel.h +++ b/include/asterisk/channel.h @@ -133,12 +133,31 @@ extern "C" { #define AST_MAX_EXTENSION 80 /*!< Max length of an extension */ #define AST_MAX_CONTEXT 80 /*!< Max length of a context */ -#define AST_MAX_UNIQUEID 150 /*!< Max length of a channel uniqueid */ -/* 150 = 127 (max systemname) + "-" + 10 (epoch - * timestamp) + "." + 10 (monotonically incrementing - * integer) + NULL. Note that if this value is ever - * changed, MAX_CHANNEL_ID should be updated in - * rtp_engine.h */ + +/*! + * Max length of a channel uniqueid reported to the outside world. + * + * \details + * 149 = 127 (max systemname) + "-" + 10 (epoch timestamp) + * + "." + 10 (monotonically incrementing integer). + * + * \note If this value is ever changed, MAX_CHANNEL_ID should + * be updated in rtp_engine.h. + */ +#define AST_MAX_PUBLIC_UNIQUEID 149 + +/*! + * Maximum size of an internal Asterisk channel unique ID. + * + * \details + * Add two for the Local;2 channel to append a ';2' if needed + * plus nul terminator. + * + * \note If this value is ever changed, MAX_CHANNEL_ID should + * be updated in rtp_engine.h. + */ +#define AST_MAX_UNIQUEID (AST_MAX_PUBLIC_UNIQUEID + 2 + 1) + #define AST_MAX_ACCOUNT_CODE 20 /*!< Max length of an account code */ #define AST_CHANNEL_NAME 80 /*!< Max length of an ast_channel name */ #define MAX_LANGUAGE 40 /*!< Max length of the language setting */ diff --git a/include/asterisk/rtp_engine.h b/include/asterisk/rtp_engine.h index 9d6c7b4f4..677a59c31 100644 --- a/include/asterisk/rtp_engine.h +++ b/include/asterisk/rtp_engine.h @@ -86,11 +86,14 @@ extern "C" { /* Maximum number of generations */ #define AST_RED_MAX_GENERATION 5 -/* Maximum size of an Asterisk channel unique ID. Should match AST_MAX_UNIQUEID. - * Note that we don't use that defined value directly here to avoid a hard dependency - * on channel.h +/*! + * Maximum size of an internal Asterisk channel unique ID. + * + * \note Must match the AST_MAX_UNIQUEID(AST_MAX_PUBLIC_UNIQUEID) value. + * We don't use that defined value directly here to avoid a hard + * dependency on channel.h. */ -#define MAX_CHANNEL_ID 150 +#define MAX_CHANNEL_ID 152 struct ast_rtp_instance; struct ast_rtp_glue; diff --git a/main/channel_internal_api.c b/main/channel_internal_api.c index d444d7fba..9cc99f3d5 100644 --- a/main/channel_internal_api.c +++ b/main/channel_internal_api.c @@ -1485,8 +1485,8 @@ void ast_channel_internal_swap_uniqueid_and_linkedid(struct ast_channel *a, stru void ast_channel_internal_set_fake_ids(struct ast_channel *chan, const char *uniqueid, const char *linkedid) { - strncpy(chan->uniqueid.unique_id, uniqueid, sizeof(chan->uniqueid.unique_id)); - strncpy(chan->linkedid.unique_id, linkedid, sizeof(chan->linkedid.unique_id)); + ast_copy_string(chan->uniqueid.unique_id, uniqueid, sizeof(chan->uniqueid.unique_id)); + ast_copy_string(chan->linkedid.unique_id, linkedid, sizeof(chan->linkedid.unique_id)); } void ast_channel_internal_cleanup(struct ast_channel *chan) diff --git a/main/core_unreal.c b/main/core_unreal.c index bc081f5f4..51f03eab3 100644 --- a/main/core_unreal.c +++ b/main/core_unreal.c @@ -907,7 +907,10 @@ struct ast_channel *ast_unreal_new_channels(struct ast_unreal_pvt *p, /* if id1 given but not id2, use default of id1;2 */ if (id1.uniqueid && ast_strlen_zero(id2.uniqueid)) { char *uniqueid2; - ast_asprintf(&uniqueid2, "%s;2", id1.uniqueid); + + uniqueid2 = ast_alloca(strlen(id1.uniqueid) + 2); + strcpy(uniqueid2, id1.uniqueid);/* Safe */ + strcat(uniqueid2, ";2");/* Safe */ id2.uniqueid = uniqueid2; } diff --git a/main/dial.c b/main/dial.c index 246d4f406..bf0d51c5e 100644 --- a/main/dial.c +++ b/main/dial.c @@ -266,8 +266,7 @@ int ast_dial_append(struct ast_dial *dial, const char *tech, const char *device, channel->device = ast_strdup(device); /* Store the assigned id */ - if (assignedids && !ast_strlen_zero(assignedids->uniqueid)) - { + if (assignedids && !ast_strlen_zero(assignedids->uniqueid)) { channel->assignedid1 = ast_strdup(assignedids->uniqueid); if (!ast_strlen_zero(assignedids->uniqueid2)) { @@ -293,7 +292,10 @@ static int begin_dial_prerun(struct ast_dial_channel *channel, struct ast_channe char numsubst[AST_MAX_EXTENSION]; struct ast_format_cap *cap_all_audio = NULL; struct ast_format_cap *cap_request; - struct ast_assigned_ids assignedids = {channel->assignedid1, channel->assignedid2}; + struct ast_assigned_ids assignedids = { + .uniqueid = channel->assignedid1, + .uniqueid2 = channel->assignedid2, + }; /* Copy device string over */ ast_copy_string(numsubst, channel->device, sizeof(numsubst)); @@ -474,14 +476,10 @@ static int handle_call_forward(struct ast_dial *dial, struct ast_dial_channel *c /* Drop old destination information */ ast_free(channel->tech); ast_free(channel->device); - if (channel->assignedid1) { - ast_free(channel->assignedid1); - channel->assignedid1 = NULL; - } - if (channel->assignedid2) { - ast_free(channel->assignedid2); - channel->assignedid2 = NULL; - } + ast_free(channel->assignedid1); + channel->assignedid1 = NULL; + ast_free(channel->assignedid2); + channel->assignedid2 = NULL; /* Update the dial channel with the new destination information */ channel->tech = ast_strdup(tech); @@ -1066,12 +1064,8 @@ int ast_dial_destroy(struct ast_dial *dial) /* Free structure */ ast_free(channel->tech); ast_free(channel->device); - if (channel->assignedid1) { - ast_free(channel->assignedid1); - } - if (channel->assignedid2) { - ast_free(channel->assignedid2); - } + ast_free(channel->assignedid1); + ast_free(channel->assignedid2); AST_LIST_REMOVE_CURRENT(list); ast_free(channel); diff --git a/main/manager.c b/main/manager.c index 3c98185be..328878251 100644 --- a/main/manager.c +++ b/main/manager.c @@ -4416,7 +4416,10 @@ static void *fast_originate(void *data) int reason = 0; struct ast_channel *chan = NULL, *chans[1]; char requested_channel[AST_CHANNEL_NAME]; - struct ast_assigned_ids assignedids = {in->channelid, in->otherchannelid}; + struct ast_assigned_ids assignedids = { + .uniqueid = in->channelid, + .uniqueid2 = in->otherchannelid + }; if (!ast_strlen_zero(in->app)) { res = ast_pbx_outgoing_app(in->tech, in->cap, in->data, @@ -4711,8 +4714,8 @@ static int action_originate(struct mansession *s, const struct message *m) const char *codecs = astman_get_header(m, "Codecs"); const char *early_media = astman_get_header(m, "Earlymedia"); struct ast_assigned_ids assignedids = { - astman_get_header(m, "ChannelId"), - astman_get_header(m, "OtherChannelId") + .uniqueid = astman_get_header(m, "ChannelId"), + .uniqueid2 = astman_get_header(m, "OtherChannelId"), }; struct ast_variable *vars = NULL; char *tech, *data; @@ -4728,17 +4731,20 @@ static int action_originate(struct mansession *s, const struct message *m) pthread_t th; int bridge_early = 0; - if ((!ast_strlen_zero(assignedids.uniqueid) && strlen(assignedids.uniqueid) >= AST_MAX_UNIQUEID) || - (!ast_strlen_zero(assignedids.uniqueid2) && strlen(assignedids.uniqueid2) >= AST_MAX_UNIQUEID)) { - ast_log(LOG_WARNING, "Uniqueid length exceeds maximum of %d\n", AST_MAX_UNIQUEID); - } - if (!cap) { astman_send_error(s, m, "Internal Error. Memory allocation failure."); return 0; } ast_format_cap_add(cap, ast_format_set(&tmp_fmt, AST_FORMAT_SLINEAR, 0)); + if ((assignedids.uniqueid && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid)) + || (assignedids.uniqueid2 && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid2))) { + astman_send_error_va(s, m, "Uniqueid length exceeds maximum of %d\n", + AST_MAX_PUBLIC_UNIQUEID); + res = 0; + goto fast_orig_cleanup; + } + if (ast_strlen_zero(name)) { astman_send_error(s, m, "Channel not specified"); res = 0; diff --git a/res/ari/resource_channels.c b/res/ari/resource_channels.c index 0f4486b52..7b7e80739 100644 --- a/res/ari/resource_channels.c +++ b/res/ari/resource_channels.c @@ -776,12 +776,10 @@ static void ari_channels_handle_originate_with_id(const char *args_endpoint, char *stuff; struct ast_channel *chan; RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL, ao2_cleanup); - struct ast_assigned_ids assignedids = {args_channel_id, args_other_channel_id}; - - if ((!ast_strlen_zero(assignedids.uniqueid) && strlen(assignedids.uniqueid) >= AST_MAX_UNIQUEID) || - (!ast_strlen_zero(assignedids.uniqueid2) && strlen(assignedids.uniqueid2) >= AST_MAX_UNIQUEID)) { - ast_log(LOG_WARNING, "Uniqueid length exceeds maximum of %d\n", AST_MAX_UNIQUEID); - } + struct ast_assigned_ids assignedids = { + .uniqueid = args_channel_id, + .uniqueid2 = args_other_channel_id, + }; if (!cap) { ast_ari_response_alloc_failed(response); @@ -789,6 +787,13 @@ static void ari_channels_handle_originate_with_id(const char *args_endpoint, } ast_format_cap_add(cap, ast_format_set(&tmp_fmt, AST_FORMAT_SLINEAR, 0)); + if ((assignedids.uniqueid && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid)) + || (assignedids.uniqueid2 && AST_MAX_PUBLIC_UNIQUEID < strlen(assignedids.uniqueid2))) { + ast_ari_response_error(response, 400, "Bad Request", + "Uniqueid length exceeds maximum of %d\n", AST_MAX_PUBLIC_UNIQUEID); + return; + } + if (ast_strlen_zero(args_endpoint)) { ast_ari_response_error(response, 400, "Bad Request", "Endpoint must be specified"); diff --git a/res/res_stasis_snoop.c b/res/res_stasis_snoop.c index eaa21603a..169e02b75 100644 --- a/res/res_stasis_snoop.c +++ b/res/res_stasis_snoop.c @@ -290,7 +290,9 @@ struct ast_channel *stasis_app_control_snoop(struct ast_channel *chan, { RAII_VAR(struct stasis_app_snoop *, snoop, NULL, ao2_cleanup); pthread_t thread; - struct ast_assigned_ids assignedids = {snoop_id, NULL}; + struct ast_assigned_ids assignedids = { + .uniqueid = snoop_id, + }; if (spy == STASIS_SNOOP_DIRECTION_NONE && whisper == STASIS_SNOOP_DIRECTION_NONE) { |