From 51cba591e305672c9576ec870166a5cfecb36641 Mon Sep 17 00:00:00 2001 From: Sean Bright <sean.bright@gmail.com> Date: Tue, 29 Sep 2020 14:04:48 -0400 Subject: [PATCH] pbx.c: On error, ast_add_extension2_lockopt should always free 'data' In the event that the desired extension already exists, ast_add_extension2_lockopt() will free the 'data' it is passed before returning an error, so we should not be freeing it ourselves. Additionally, there were two places where ast_add_extension2_lockopt() could return an error without also freeing the 'data' pointer, so we add that. ASTERISK-29097 #close Change-Id: I904707aae55169feda050a5ed7c6793b53fe6eae --- include/asterisk/pbx.h | 8 ++++++-- main/pbx.c | 12 +++++++++++- res/parking/parking_bridge_features.c | 1 - res/res_parking.c | 1 - res/res_pjsip_config_wizard.c | 1 - 5 files changed, 17 insertions(+), 6 deletions(-) diff --git a/include/asterisk/pbx.h b/include/asterisk/pbx.h index cbc5ec2320..c75e50a3ba 100644 --- a/include/asterisk/pbx.h +++ b/include/asterisk/pbx.h @@ -492,9 +492,13 @@ void ast_pbx_hangup_handler_push(struct ast_channel *chan, const char *handler); * \param callerid pattern to match CallerID, or NULL to match any CallerID * \param application application to run on the extension with that priority level * \param data data to pass to the application - * \param datad + * \param datad a pointer to a function that will deallocate \c data when needed + * or NULL if \c data does not need to be freed. * \param registrar who registered the extension * + * \note On any failure, the function pointed to by \c datap will be called and passed the + * \c data pointer. + * * \retval 0 success * \retval -1 failure */ @@ -520,7 +524,7 @@ int ast_add_extension2(struct ast_context *con, int replace, const char *extensi * \since 12.0.0 * * \note con must be write locked prior to calling. For details about the arguments, - * check ast_add_extension2() + * check ast_add_extension() */ int ast_add_extension2_nolock(struct ast_context *con, int replace, const char *extension, int priority, const char *label, const char *callerid, diff --git a/main/pbx.c b/main/pbx.c index b520f5fc98..0609240a32 100644 --- a/main/pbx.c +++ b/main/pbx.c @@ -7346,6 +7346,10 @@ static int ast_add_extension2_lockopt(struct ast_context *con, if (ast_strlen_zero(extension)) { ast_log(LOG_ERROR,"You have to be kidding-- add exten '' to context %s? Figure out a name and call me back. Action ignored.\n", con->name); + /* We always need to deallocate 'data' on failure */ + if (datad) { + datad(data); + } return -1; } @@ -7401,8 +7405,14 @@ static int ast_add_extension2_lockopt(struct ast_context *con, } /* Be optimistic: Build the extension structure first */ - if (!(tmp = ast_calloc(1, length))) + tmp = ast_calloc(1, length); + if (!tmp) { + /* We always need to deallocate 'data' on failure */ + if (datad) { + datad(data); + } return -1; + } if (ast_strlen_zero(label)) /* let's turn empty labels to a null ptr */ label = 0; diff --git a/res/parking/parking_bridge_features.c b/res/parking/parking_bridge_features.c index 5bcdb215cc..6863d52e9e 100644 --- a/res/parking/parking_bridge_features.c +++ b/res/parking/parking_bridge_features.c @@ -660,7 +660,6 @@ static int parking_duration_callback(struct ast_bridge_channel *bridge_channel, dial_string_flat, PARK_DIAL_CONTEXT, ast_get_extension_registrar(existing_exten)); } else if (ast_add_extension2_nolock(park_dial_context, 1, dial_string_flat, 1, NULL, NULL, "Dial", duplicate_returnexten, ast_free_ptr, BASE_REGISTRAR, NULL, 0)) { - ast_free(duplicate_returnexten); ast_log(LOG_ERROR, "Failed to create parking redial parker extension %s@%s - Dial(%s)\n", dial_string_flat, PARK_DIAL_CONTEXT, returnexten); } diff --git a/res/res_parking.c b/res/res_parking.c index 470396d260..cde82d2235 100644 --- a/res/res_parking.c +++ b/res/res_parking.c @@ -721,7 +721,6 @@ static int parking_add_extension(struct ast_context *context, int replace, const if (ast_add_extension2_nolock(context, replace, extension, priority, NULL, NULL, application, data_duplicate, ast_free_ptr, registrar, NULL, 0)) { - ast_free(data_duplicate); return -1; } diff --git a/res/res_pjsip_config_wizard.c b/res/res_pjsip_config_wizard.c index aec4b58ac0..e61b7c5cc8 100644 --- a/res/res_pjsip_config_wizard.c +++ b/res/res_pjsip_config_wizard.c @@ -475,7 +475,6 @@ static int add_extension(struct ast_context *context, const char *exten, if (ast_add_extension2_nolock(context, 0, exten, priority, NULL, NULL, app, data, free_ptr, BASE_REGISTRAR, NULL, 0)) { - ast_free(data); return -1; } -- GitLab