Fix unsafe uses of ast_context pointers.
Although ast_context_find, ast_context_find_or_create and ast_context_destroy perform locking of the contexts table, any context pointer can become invalid at any time that the contexts table is unlocked. This change adds locking around all complete operations involving these functions. Places where ast_context_find was followed by ast_context_destroy have been replaced with calls ast_context_destroy_by_name. ASTERISK-25094 #close Reported by: Corey Farrell Change-Id: I1866b6787730c9c4f3f836b6133ffe9c820734fa
This commit is contained in:
parent
eed9b1ce52
commit
53658a14cc
10 changed files with 91 additions and 74 deletions
|
@ -7347,14 +7347,13 @@ static int sla_build_trunk(struct ast_config *cfg, const char *cat)
|
|||
ao2_unlock(trunk);
|
||||
|
||||
if (!ast_strlen_zero(trunk->autocontext)) {
|
||||
struct ast_context *context;
|
||||
context = ast_context_find_or_create(NULL, NULL, trunk->autocontext, sla_registrar);
|
||||
if (!context) {
|
||||
if (!ast_context_find_or_create(NULL, NULL, trunk->autocontext, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically find or create "
|
||||
"context '%s' for SLA!\n", trunk->autocontext);
|
||||
return -1;
|
||||
}
|
||||
if (ast_add_extension2(context, 0 /* don't replace */, "s", 1,
|
||||
|
||||
if (ast_add_extension(trunk->autocontext, 0 /* don't replace */, "s", 1,
|
||||
NULL, NULL, slatrunk_app, ast_strdup(trunk->name), ast_free_ptr, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically create extension "
|
||||
"for trunk '%s'!\n", trunk->name);
|
||||
|
@ -7523,17 +7522,16 @@ static int sla_build_station(struct ast_config *cfg, const char *cat)
|
|||
ao2_unlock(station);
|
||||
|
||||
if (!ast_strlen_zero(station->autocontext)) {
|
||||
struct ast_context *context;
|
||||
struct sla_trunk_ref *trunk_ref;
|
||||
context = ast_context_find_or_create(NULL, NULL, station->autocontext, sla_registrar);
|
||||
if (!context) {
|
||||
|
||||
if (!ast_context_find_or_create(NULL, NULL, station->autocontext, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically find or create "
|
||||
"context '%s' for SLA!\n", station->autocontext);
|
||||
return -1;
|
||||
}
|
||||
/* The extension for when the handset goes off-hook.
|
||||
* exten => station1,1,SLAStation(station1) */
|
||||
if (ast_add_extension2(context, 0 /* don't replace */, station->name, 1,
|
||||
if (ast_add_extension(station->autocontext, 0 /* don't replace */, station->name, 1,
|
||||
NULL, NULL, slastation_app, ast_strdup(station->name), ast_free_ptr, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically create extension "
|
||||
"for trunk '%s'!\n", station->name);
|
||||
|
@ -7546,7 +7544,7 @@ static int sla_build_station(struct ast_config *cfg, const char *cat)
|
|||
snprintf(hint, sizeof(hint), "SLA:%s", exten);
|
||||
/* Extension for this line button
|
||||
* exten => station1_line1,1,SLAStation(station1_line1) */
|
||||
if (ast_add_extension2(context, 0 /* don't replace */, exten, 1,
|
||||
if (ast_add_extension(station->autocontext, 0 /* don't replace */, exten, 1,
|
||||
NULL, NULL, slastation_app, ast_strdup(exten), ast_free_ptr, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically create extension "
|
||||
"for trunk '%s'!\n", station->name);
|
||||
|
@ -7554,7 +7552,7 @@ static int sla_build_station(struct ast_config *cfg, const char *cat)
|
|||
}
|
||||
/* Hint for this line button
|
||||
* exten => station1_line1,hint,SLA:station1_line1 */
|
||||
if (ast_add_extension2(context, 0 /* don't replace */, exten, PRIORITY_HINT,
|
||||
if (ast_add_extension(station->autocontext, 0 /* don't replace */, exten, PRIORITY_HINT,
|
||||
NULL, NULL, hint, NULL, NULL, sla_registrar)) {
|
||||
ast_log(LOG_ERROR, "Failed to automatically create hint "
|
||||
"for trunk '%s'!\n", station->name);
|
||||
|
|
|
@ -14783,7 +14783,6 @@ static void cleanup_thread_list(void *head)
|
|||
|
||||
static int __unload_module(void)
|
||||
{
|
||||
struct ast_context *con;
|
||||
int x;
|
||||
|
||||
network_change_event_unsubscribe();
|
||||
|
@ -14863,9 +14862,7 @@ static int __unload_module(void)
|
|||
ao2_ref(callno_pool, -1);
|
||||
ao2_ref(callno_pool_trunk, -1);
|
||||
|
||||
con = ast_context_find(regcontext);
|
||||
if (con)
|
||||
ast_context_destroy(con, "IAX2");
|
||||
ast_context_destroy_by_name(regcontext, "IAX2");
|
||||
ast_unload_realtime("iaxpeers");
|
||||
|
||||
iax2_tech.capabilities = ast_format_cap_destroy(iax2_tech.capabilities);
|
||||
|
|
|
@ -19509,8 +19509,7 @@ static void cleanup_stale_contexts(char *new, char *old)
|
|||
}
|
||||
|
||||
}
|
||||
if (stalecontext)
|
||||
ast_context_destroy(ast_context_find(stalecontext), "SIP");
|
||||
ast_context_destroy_by_name(stalecontext, "SIP");
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -34869,7 +34868,6 @@ static int unload_module(void)
|
|||
{
|
||||
struct sip_pvt *p;
|
||||
struct sip_threadinfo *th;
|
||||
struct ast_context *con;
|
||||
struct ao2_iterator i;
|
||||
int wait_count;
|
||||
|
||||
|
@ -35045,10 +35043,7 @@ static int unload_module(void)
|
|||
close(sipsock);
|
||||
io_context_destroy(io);
|
||||
ast_sched_context_destroy(sched);
|
||||
con = ast_context_find(used_context);
|
||||
if (con) {
|
||||
ast_context_destroy(con, "SIP");
|
||||
}
|
||||
ast_context_destroy_by_name(used_context, "SIP");
|
||||
ast_unload_realtime("sipregs");
|
||||
ast_unload_realtime("sippeers");
|
||||
ast_cc_monitor_unregister(&sip_cc_monitor_callbacks);
|
||||
|
|
|
@ -2037,8 +2037,7 @@ static void cleanup_stale_contexts(char *new, char *old)
|
|||
}
|
||||
|
||||
}
|
||||
if (stalecontext)
|
||||
ast_context_destroy(ast_context_find(stalecontext), "Skinny");
|
||||
ast_context_destroy_by_name(stalecontext, "Skinny");
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -8014,7 +8013,6 @@ static int unload_module(void)
|
|||
struct skinny_device *d;
|
||||
struct skinny_line *l;
|
||||
struct skinny_subchannel *sub;
|
||||
struct ast_context *con;
|
||||
|
||||
ast_rtp_glue_unregister(&skinny_rtp_glue);
|
||||
ast_channel_unregister(&skinny_tech);
|
||||
|
@ -8069,9 +8067,7 @@ static int unload_module(void)
|
|||
ast_sched_context_destroy(sched);
|
||||
}
|
||||
|
||||
con = ast_context_find(used_context);
|
||||
if (con)
|
||||
ast_context_destroy(con, "Skinny");
|
||||
ast_context_destroy_by_name(used_context, "Skinny");
|
||||
|
||||
default_cap = ast_format_cap_destroy(default_cap);
|
||||
skinny_tech.capabilities = ast_format_cap_destroy(skinny_tech.capabilities);
|
||||
|
|
|
@ -291,7 +291,21 @@ struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts,
|
|||
void ast_merge_contexts_and_delete(struct ast_context **extcontexts, struct ast_hashtab *exttable, const char *registrar);
|
||||
|
||||
/*!
|
||||
* \brief Destroy a context (matches the specified context (or ANY context if NULL)
|
||||
* \brief Destroy a context by name
|
||||
*
|
||||
* \param context Name of the context to destroy
|
||||
* \param registrar who registered it
|
||||
*
|
||||
* You can optionally leave out the registrar parameter. It will find it
|
||||
* based on the context name.
|
||||
*
|
||||
* \retval -1 context not found
|
||||
* \retval 0 Success
|
||||
*/
|
||||
int ast_context_destroy_by_name(const char *context, const char *registrar);
|
||||
|
||||
/*!
|
||||
* \brief Destroy a context (matches the specified context or ANY context if NULL)
|
||||
*
|
||||
* \param con context to destroy
|
||||
* \param registrar who registered it
|
||||
|
|
|
@ -5271,6 +5271,7 @@ static void manage_parkinglot(struct ast_parkinglot *curlot, const struct pollfd
|
|||
}
|
||||
if (manage_parked_call(pu, pfds, nfds, new_pfds, new_nfds, ms)) {
|
||||
/* Parking is complete for this call so remove it from the parking lot. */
|
||||
ast_wrlock_contexts();
|
||||
con = ast_context_find(pu->parkinglot->cfg.parking_con);
|
||||
if (con) {
|
||||
if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 0)) {
|
||||
|
@ -5278,6 +5279,9 @@ static void manage_parkinglot(struct ast_parkinglot *curlot, const struct pollfd
|
|||
"Whoa, failed to remove the parking extension %s@%s!\n",
|
||||
pu->parkingexten, pu->parkinglot->cfg.parking_con);
|
||||
}
|
||||
}
|
||||
ast_unlock_contexts();
|
||||
if (con) {
|
||||
notify_metermaids(pu->parkingexten, pu->parkinglot->cfg.parking_con,
|
||||
AST_DEVICE_NOT_INUSE);
|
||||
} else {
|
||||
|
@ -5493,7 +5497,7 @@ static int park_call_exec(struct ast_channel *chan, const char *data)
|
|||
/*! \brief Pickup parked call */
|
||||
static int parked_call_exec(struct ast_channel *chan, const char *data)
|
||||
{
|
||||
int res;
|
||||
int res = 0;
|
||||
struct ast_channel *peer = NULL;
|
||||
struct parkeduser *pu;
|
||||
struct ast_context *con;
|
||||
|
@ -5567,9 +5571,15 @@ static int parked_call_exec(struct ast_channel *chan, const char *data)
|
|||
callid = ast_callid_unref(callid);
|
||||
}
|
||||
|
||||
ast_wrlock_contexts();
|
||||
con = ast_context_find(parkinglot->cfg.parking_con);
|
||||
if (con) {
|
||||
if (ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 0)) {
|
||||
res = ast_context_remove_extension2(con, pu->parkingexten, 1, NULL, 1);
|
||||
}
|
||||
ast_unlock_contexts();
|
||||
|
||||
if (con) {
|
||||
if (res) {
|
||||
ast_log(LOG_WARNING, "Whoa, failed to remove the extension!\n");
|
||||
} else {
|
||||
notify_metermaids(pu->parkingexten, parkinglot->cfg.parking_con, AST_DEVICE_NOT_INUSE);
|
||||
|
@ -6978,7 +6988,6 @@ static void remove_dead_dialplan_useage(struct parking_dp_map *old_map, struct p
|
|||
{
|
||||
struct parking_dp_context *old_ctx;
|
||||
struct parking_dp_context *new_ctx;
|
||||
struct ast_context *con;
|
||||
int cmp;
|
||||
|
||||
old_ctx = AST_LIST_FIRST(old_map);
|
||||
|
@ -6992,10 +7001,7 @@ static void remove_dead_dialplan_useage(struct parking_dp_map *old_map, struct p
|
|||
cmp = strcmp(old_ctx->context, new_ctx->context);
|
||||
if (cmp < 0) {
|
||||
/* New map does not have old map context. */
|
||||
con = ast_context_find(old_ctx->context);
|
||||
if (con) {
|
||||
ast_context_destroy(con, registrar);
|
||||
}
|
||||
ast_context_destroy_by_name(old_ctx->context, registrar);
|
||||
old_ctx = AST_LIST_NEXT(old_ctx, node);
|
||||
continue;
|
||||
}
|
||||
|
@ -7011,10 +7017,7 @@ static void remove_dead_dialplan_useage(struct parking_dp_map *old_map, struct p
|
|||
|
||||
/* Any old contexts left must be dead. */
|
||||
for (; old_ctx; old_ctx = AST_LIST_NEXT(old_ctx, node)) {
|
||||
con = ast_context_find(old_ctx->context);
|
||||
if (con) {
|
||||
ast_context_destroy(con, registrar);
|
||||
}
|
||||
ast_context_destroy_by_name(old_ctx->context, registrar);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -7245,7 +7248,6 @@ static char *handle_feature_show(struct ast_cli_entry *e, int cmd, struct ast_cl
|
|||
|
||||
int ast_features_reload(void)
|
||||
{
|
||||
struct ast_context *con;
|
||||
int res;
|
||||
|
||||
ast_mutex_lock(&features_reload_lock);/* Searialize reloading features.conf */
|
||||
|
@ -7258,10 +7260,7 @@ int ast_features_reload(void)
|
|||
* extension. This is a small window of opportunity on an
|
||||
* execution chain that is not expected to happen very often.
|
||||
*/
|
||||
con = ast_context_find(parking_con_dial);
|
||||
if (con) {
|
||||
ast_context_destroy(con, registrar);
|
||||
}
|
||||
ast_context_destroy_by_name(parking_con_dial, registrar);
|
||||
|
||||
res = load_config(1);
|
||||
ast_mutex_unlock(&features_reload_lock);
|
||||
|
@ -8687,9 +8686,15 @@ static int unpark_test_channel(struct ast_channel *toremove, struct ast_park_cal
|
|||
return -1;
|
||||
}
|
||||
|
||||
ast_wrlock_contexts();
|
||||
con = ast_context_find(args->pu->parkinglot->cfg.parking_con);
|
||||
if (con) {
|
||||
if (ast_context_remove_extension2(con, args->pu->parkingexten, 1, NULL, 0)) {
|
||||
res = ast_context_remove_extension2(con, args->pu->parkingexten, 1, NULL, 1);
|
||||
}
|
||||
ast_unlock_contexts();
|
||||
|
||||
if (con) {
|
||||
if (res) {
|
||||
ast_log(LOG_WARNING, "Whoa, failed to remove the parking extension!\n");
|
||||
res = -1;
|
||||
} else {
|
||||
|
|
36
main/pbx.c
36
main/pbx.c
|
@ -8784,9 +8784,9 @@ struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts,
|
|||
ast_rdlock_contexts();
|
||||
local_contexts = &contexts;
|
||||
tmp = ast_hashtab_lookup(contexts_table, &search);
|
||||
ast_unlock_contexts();
|
||||
if (tmp) {
|
||||
tmp->refcount++;
|
||||
ast_unlock_contexts();
|
||||
return tmp;
|
||||
}
|
||||
} else { /* local contexts just in a linked list; search there for the new context; slow, linear search, but not frequent */
|
||||
|
@ -8810,11 +8810,13 @@ struct ast_context *ast_context_find_or_create(struct ast_context **extcontexts,
|
|||
tmp->refcount = 1;
|
||||
} else {
|
||||
ast_log(LOG_ERROR, "Danger! We failed to allocate a context for %s!\n", name);
|
||||
if (!extcontexts) {
|
||||
ast_unlock_contexts();
|
||||
}
|
||||
return NULL;
|
||||
}
|
||||
|
||||
if (!extcontexts) {
|
||||
ast_wrlock_contexts();
|
||||
tmp->next = *local_contexts;
|
||||
*local_contexts = tmp;
|
||||
ast_hashtab_insert_safe(contexts_table, tmp); /*put this context into the tree */
|
||||
|
@ -9710,18 +9712,24 @@ int ast_context_add_ignorepat2(struct ast_context *con, const char *value, const
|
|||
|
||||
int ast_ignore_pattern(const char *context, const char *pattern)
|
||||
{
|
||||
struct ast_context *con = ast_context_find(context);
|
||||
int ret = 0;
|
||||
struct ast_context *con;
|
||||
|
||||
ast_rdlock_contexts();
|
||||
con = ast_context_find(context);
|
||||
if (con) {
|
||||
struct ast_ignorepat *pat;
|
||||
|
||||
for (pat = con->ignorepats; pat; pat = pat->next) {
|
||||
if (ast_extension_match(pat->pattern, pattern))
|
||||
return 1;
|
||||
if (ast_extension_match(pat->pattern, pattern)) {
|
||||
ret = 1;
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
ast_unlock_contexts();
|
||||
|
||||
return 0;
|
||||
return ret;
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -11035,6 +11043,22 @@ void __ast_context_destroy(struct ast_context *list, struct ast_hashtab *context
|
|||
}
|
||||
}
|
||||
|
||||
int ast_context_destroy_by_name(const char *context, const char *registrar)
|
||||
{
|
||||
struct ast_context *con;
|
||||
int ret = -1;
|
||||
|
||||
ast_wrlock_contexts();
|
||||
con = ast_context_find(context);
|
||||
if (con) {
|
||||
ast_context_destroy(con, registrar);
|
||||
ret = 0;
|
||||
}
|
||||
ast_unlock_contexts();
|
||||
|
||||
return ret;
|
||||
}
|
||||
|
||||
void ast_context_destroy(struct ast_context *con, const char *registrar)
|
||||
{
|
||||
ast_wrlock_contexts();
|
||||
|
|
|
@ -80,8 +80,6 @@ static char *complete_dialplan_remove_context(struct ast_cli_args *);
|
|||
|
||||
static char *handle_cli_dialplan_remove_context(struct ast_cli_entry *e, int cmd, struct ast_cli_args *a)
|
||||
{
|
||||
struct ast_context *con;
|
||||
|
||||
switch (cmd) {
|
||||
case CLI_INIT:
|
||||
e->command = "dialplan remove context";
|
||||
|
@ -97,16 +95,11 @@ static char *handle_cli_dialplan_remove_context(struct ast_cli_entry *e, int cmd
|
|||
return CLI_SHOWUSAGE;
|
||||
}
|
||||
|
||||
con = ast_context_find(a->argv[3]);
|
||||
|
||||
if (!con) {
|
||||
ast_cli(a->fd, "There is no such context as '%s'\n",
|
||||
a->argv[3]);
|
||||
return CLI_SUCCESS;
|
||||
if (ast_context_destroy_by_name(a->argv[3], NULL)) {
|
||||
ast_cli(a->fd, "There is no such context as '%s'\n", a->argv[3]);
|
||||
return CLI_SUCCESS;
|
||||
} else {
|
||||
ast_context_destroy(con, registrar);
|
||||
ast_cli(a->fd, "Removing context '%s'\n",
|
||||
a->argv[3]);
|
||||
ast_cli(a->fd, "Removed context '%s'\n", a->argv[3]);
|
||||
return CLI_SUCCESS;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -42,10 +42,10 @@ ASTERISK_FILE_VERSION(__FILE__, "$Revision$")
|
|||
|
||||
AST_TEST_DEFINE(test_gosub)
|
||||
{
|
||||
#define CONTEXT_NAME "tests_test_gosub_virtual_context"
|
||||
int res = AST_TEST_PASS, i;
|
||||
struct ast_channel *chan;
|
||||
struct ast_str *str;
|
||||
struct ast_context *con;
|
||||
struct testplan {
|
||||
const char *app;
|
||||
const char *args;
|
||||
|
@ -119,14 +119,14 @@ AST_TEST_DEFINE(test_gosub)
|
|||
}
|
||||
|
||||
/* Create our test dialplan */
|
||||
if (!(con = ast_context_find_or_create(NULL, NULL, "tests_test_gosub_virtual_context", "test_gosub"))) {
|
||||
if (!ast_context_find_or_create(NULL, NULL, CONTEXT_NAME, "test_gosub")) {
|
||||
ast_test_status_update(test, "Unable to create test dialplan context");
|
||||
ast_free(str);
|
||||
ast_channel_unref(chan);
|
||||
return AST_TEST_FAIL;
|
||||
}
|
||||
|
||||
ast_add_extension2(con, 1, "s", 1, NULL, NULL, "NoOp", ast_strdup(""), ast_free_ptr, "test_gosub");
|
||||
ast_add_extension(CONTEXT_NAME, 1, "s", 1, NULL, NULL, "NoOp", ast_strdup(""), ast_free_ptr, "test_gosub");
|
||||
|
||||
for (i = 0; i < ARRAY_LEN(testplan); i++) {
|
||||
if (testplan[i].app == NULL) {
|
||||
|
@ -157,8 +157,8 @@ AST_TEST_DEFINE(test_gosub)
|
|||
|
||||
ast_free(str);
|
||||
ast_channel_unref(chan);
|
||||
ast_context_remove_extension2(con, "s", 1, NULL, 0);
|
||||
ast_context_destroy(con, "test_gosub");
|
||||
ast_context_remove_extension(CONTEXT_NAME, "s", 1, NULL);
|
||||
ast_context_destroy(NULL, "test_gosub");
|
||||
|
||||
return res;
|
||||
}
|
||||
|
|
|
@ -198,7 +198,6 @@ AST_TEST_DEFINE(pattern_match_test)
|
|||
*/
|
||||
struct {
|
||||
const char * context_string;
|
||||
struct ast_context *context;
|
||||
} contexts[] = {
|
||||
{ TEST_PATTERN, },
|
||||
{ TEST_PATTERN_INCLUDE, },
|
||||
|
@ -267,7 +266,7 @@ AST_TEST_DEFINE(pattern_match_test)
|
|||
*/
|
||||
|
||||
for (i = 0; i < ARRAY_LEN(contexts); ++i) {
|
||||
if (!(contexts[i].context = ast_context_find_or_create(NULL, NULL, contexts[i].context_string, registrar))) {
|
||||
if (!ast_context_find_or_create(NULL, NULL, contexts[i].context_string, registrar)) {
|
||||
ast_test_status_update(test, "Failed to create context %s\n", contexts[i].context_string);
|
||||
res = AST_TEST_FAIL;
|
||||
goto cleanup;
|
||||
|
@ -319,11 +318,7 @@ AST_TEST_DEFINE(pattern_match_test)
|
|||
}
|
||||
|
||||
cleanup:
|
||||
for (i = 0; i < ARRAY_LEN(contexts); ++i) {
|
||||
if (contexts[i].context) {
|
||||
ast_context_destroy(contexts[i].context, registrar);
|
||||
}
|
||||
}
|
||||
ast_context_destroy(NULL, registrar);
|
||||
|
||||
return res;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue