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
This commit is contained in:
Richard Mudgett
2014-03-20 16:35:57 +00:00
parent 57239bfe37
commit 1ba13718fc
8 changed files with 77 additions and 45 deletions

View File

@@ -133,12 +133,31 @@ extern "C" {
#define AST_MAX_EXTENSION 80 /*!< Max length of an extension */ #define AST_MAX_EXTENSION 80 /*!< Max length of an extension */
#define AST_MAX_CONTEXT 80 /*!< Max length of a context */ #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 * Max length of a channel uniqueid reported to the outside world.
* integer) + NULL. Note that if this value is ever *
* changed, MAX_CHANNEL_ID should be updated in * \details
* rtp_engine.h */ * 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_MAX_ACCOUNT_CODE 20 /*!< Max length of an account code */
#define AST_CHANNEL_NAME 80 /*!< Max length of an ast_channel name */ #define AST_CHANNEL_NAME 80 /*!< Max length of an ast_channel name */
#define MAX_LANGUAGE 40 /*!< Max length of the language setting */ #define MAX_LANGUAGE 40 /*!< Max length of the language setting */

View File

@@ -86,11 +86,14 @@ extern "C" {
/* Maximum number of generations */ /* Maximum number of generations */
#define AST_RED_MAX_GENERATION 5 #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 * Maximum size of an internal Asterisk channel unique ID.
* on channel.h *
* \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_instance;
struct ast_rtp_glue; struct ast_rtp_glue;

View File

@@ -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) 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)); ast_copy_string(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->linkedid.unique_id, linkedid, sizeof(chan->linkedid.unique_id));
} }
void ast_channel_internal_cleanup(struct ast_channel *chan) void ast_channel_internal_cleanup(struct ast_channel *chan)

View File

@@ -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 given but not id2, use default of id1;2 */
if (id1.uniqueid && ast_strlen_zero(id2.uniqueid)) { if (id1.uniqueid && ast_strlen_zero(id2.uniqueid)) {
char *uniqueid2; 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; id2.uniqueid = uniqueid2;
} }

View File

@@ -266,8 +266,7 @@ int ast_dial_append(struct ast_dial *dial, const char *tech, const char *device,
channel->device = ast_strdup(device); channel->device = ast_strdup(device);
/* Store the assigned id */ /* Store the assigned id */
if (assignedids && !ast_strlen_zero(assignedids->uniqueid)) if (assignedids && !ast_strlen_zero(assignedids->uniqueid)) {
{
channel->assignedid1 = ast_strdup(assignedids->uniqueid); channel->assignedid1 = ast_strdup(assignedids->uniqueid);
if (!ast_strlen_zero(assignedids->uniqueid2)) { 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]; char numsubst[AST_MAX_EXTENSION];
struct ast_format_cap *cap_all_audio = NULL; struct ast_format_cap *cap_all_audio = NULL;
struct ast_format_cap *cap_request; 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 */ /* Copy device string over */
ast_copy_string(numsubst, channel->device, sizeof(numsubst)); 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 */ /* Drop old destination information */
ast_free(channel->tech); ast_free(channel->tech);
ast_free(channel->device); ast_free(channel->device);
if (channel->assignedid1) { ast_free(channel->assignedid1);
ast_free(channel->assignedid1); channel->assignedid1 = NULL;
channel->assignedid1 = NULL; ast_free(channel->assignedid2);
} channel->assignedid2 = NULL;
if (channel->assignedid2) {
ast_free(channel->assignedid2);
channel->assignedid2 = NULL;
}
/* Update the dial channel with the new destination information */ /* Update the dial channel with the new destination information */
channel->tech = ast_strdup(tech); channel->tech = ast_strdup(tech);
@@ -1066,12 +1064,8 @@ int ast_dial_destroy(struct ast_dial *dial)
/* Free structure */ /* Free structure */
ast_free(channel->tech); ast_free(channel->tech);
ast_free(channel->device); ast_free(channel->device);
if (channel->assignedid1) { ast_free(channel->assignedid1);
ast_free(channel->assignedid1); ast_free(channel->assignedid2);
}
if (channel->assignedid2) {
ast_free(channel->assignedid2);
}
AST_LIST_REMOVE_CURRENT(list); AST_LIST_REMOVE_CURRENT(list);
ast_free(channel); ast_free(channel);

View File

@@ -4416,7 +4416,10 @@ static void *fast_originate(void *data)
int reason = 0; int reason = 0;
struct ast_channel *chan = NULL, *chans[1]; struct ast_channel *chan = NULL, *chans[1];
char requested_channel[AST_CHANNEL_NAME]; 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)) { if (!ast_strlen_zero(in->app)) {
res = ast_pbx_outgoing_app(in->tech, in->cap, in->data, 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 *codecs = astman_get_header(m, "Codecs");
const char *early_media = astman_get_header(m, "Earlymedia"); const char *early_media = astman_get_header(m, "Earlymedia");
struct ast_assigned_ids assignedids = { struct ast_assigned_ids assignedids = {
astman_get_header(m, "ChannelId"), .uniqueid = astman_get_header(m, "ChannelId"),
astman_get_header(m, "OtherChannelId") .uniqueid2 = astman_get_header(m, "OtherChannelId"),
}; };
struct ast_variable *vars = NULL; struct ast_variable *vars = NULL;
char *tech, *data; char *tech, *data;
@@ -4728,17 +4731,20 @@ static int action_originate(struct mansession *s, const struct message *m)
pthread_t th; pthread_t th;
int bridge_early = 0; 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) { if (!cap) {
astman_send_error(s, m, "Internal Error. Memory allocation failure."); astman_send_error(s, m, "Internal Error. Memory allocation failure.");
return 0; return 0;
} }
ast_format_cap_add(cap, ast_format_set(&tmp_fmt, AST_FORMAT_SLINEAR, 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)) { if (ast_strlen_zero(name)) {
astman_send_error(s, m, "Channel not specified"); astman_send_error(s, m, "Channel not specified");
res = 0; res = 0;

View File

@@ -776,12 +776,10 @@ static void ari_channels_handle_originate_with_id(const char *args_endpoint,
char *stuff; char *stuff;
struct ast_channel *chan; struct ast_channel *chan;
RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL, ao2_cleanup); RAII_VAR(struct ast_channel_snapshot *, snapshot, NULL, ao2_cleanup);
struct ast_assigned_ids assignedids = {args_channel_id, args_other_channel_id}; struct ast_assigned_ids assignedids = {
.uniqueid = args_channel_id,
if ((!ast_strlen_zero(assignedids.uniqueid) && strlen(assignedids.uniqueid) >= AST_MAX_UNIQUEID) || .uniqueid2 = args_other_channel_id,
(!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) { if (!cap) {
ast_ari_response_alloc_failed(response); 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)); 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)) { if (ast_strlen_zero(args_endpoint)) {
ast_ari_response_error(response, 400, "Bad Request", ast_ari_response_error(response, 400, "Bad Request",
"Endpoint must be specified"); "Endpoint must be specified");

View File

@@ -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); RAII_VAR(struct stasis_app_snoop *, snoop, NULL, ao2_cleanup);
pthread_t thread; 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 && if (spy == STASIS_SNOOP_DIRECTION_NONE &&
whisper == STASIS_SNOOP_DIRECTION_NONE) { whisper == STASIS_SNOOP_DIRECTION_NONE) {