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

diff --git a/apps/app_meetme.c b/apps/app_meetme.c
index e74ad751871a87742c29e7cf853b05f5c2deee3e..0c339d6ba50cfb755aff8f0a871a236f839e8771 100644 (file)
--- a/apps/app_meetme.c
+++ b/apps/app_meetme.c
@@ -7539,14 +7539,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);
@@ -7715,17 +7714,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);
@@ -7738,7 +7736,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);
@@ -7746,7 +7744,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);

diff --git a/channels/chan_iax2.c b/channels/chan_iax2.c
index dbad79dcc206942b5982a2275672f019c7ce6e04..c797b87505f1153e79caf7e5f3a286d0ea048666 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -14626,7 +14626,6 @@ static void cleanup_thread_list(void *head)
 
 static int __unload_module(void)
 {
-       struct ast_context *con;
        int x;
 
        network_change_stasis_unsubscribe();
@@ -14703,9 +14702,7 @@ static int __unload_module(void)
        sched = NULL;
        ao2_ref(peercnts, -1);
 
-       con = ast_context_find(regcontext);
-       if (con)
-               ast_context_destroy(con, "IAX2");
+       ast_context_destroy_by_name(regcontext, "IAX2");
        ast_unload_realtime("iaxpeers");
 
        ao2_ref(iax2_tech.capabilities, -1);

diff --git a/channels/chan_sip.c b/channels/chan_sip.c
index 7c4c8a6113ca0349b74429e917fd5dbbdd979434..1d58a844f13d3805cf2c387f42b09faa39861b60 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -19740,8 +19740,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");
        }
 }
 
@@ -34550,7 +34549,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;
 
@@ -34730,10 +34728,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);

diff --git a/channels/chan_skinny.c b/channels/chan_skinny.c
index 03da0e0d8b6b140f4c43f96cac54b37575a89c93..08629fd78323bf91673c230b64f00a4d04fbff37 100644 (file)
--- a/channels/chan_skinny.c
+++ b/channels/chan_skinny.c
@@ -2198,8 +2198,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");
        }
 }
 
@@ -8717,7 +8716,6 @@ static int unload_module(void)
        struct skinny_device *d;
        struct skinny_line *l;
        struct skinny_subchannel *sub;
-       struct ast_context *con;
        pthread_t tempthread;
 
        ast_rtp_glue_unregister(&skinny_rtp_glue);
@@ -8778,9 +8776,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");
 
        ao2_ref(default_cap, -1);
        return 0;

diff --git a/include/asterisk/pbx.h b/include/asterisk/pbx.h
index c09de982a64bd5eefe200ba88738f7d39be680db..f5feb9366ae41cd765c8c7f41a0394a3395bfceb 100644 (file)
--- a/include/asterisk/pbx.h
+++ b/include/asterisk/pbx.h
@@ -300,7 +300,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

diff --git a/main/pbx.c b/main/pbx.c
index 45909f5d95f2ff406d23a3cdc6d58e93d001984e..0da0fef1e28fa2289600a5876ff6c1d8c12fd283 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -8739,9 +8739,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 */
@@ -8765,11 +8765,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 */
@@ -9668,18 +9670,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;
 }
 
 /*
@@ -10887,6 +10895,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();

diff --git a/pbx/pbx_config.c b/pbx/pbx_config.c
index d85b901f9f6e63632bca2dc9f736b2f51d50d5ed..5848e91d86cc4b215312ac9082bf730334f07acc 100644 (file)
--- a/pbx/pbx_config.c
+++ b/pbx/pbx_config.c
@@ -133,8 +133,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";
@@ -150,16 +148,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;
        }
 }

diff --git a/tests/test_gosub.c b/tests/test_gosub.c
index c4b071ee29cdbe2638a5bf700bf6feb6c77e8da6..e0f618cd5f813af75a066461eb833c2ea950cf0c 100644 (file)
--- a/tests/test_gosub.c
+++ b/tests/test_gosub.c
@@ -42,10 +42,10 @@ ASTERISK_REGISTER_FILE()
 
 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;
 }

diff --git a/tests/test_pbx.c b/tests/test_pbx.c
index 388baa3f5b0f217487a8cbe8909674ad417a7750..88451672d6ef9b23349334482236a4a350789654 100644 (file)
--- a/tests/test_pbx.c
+++ b/tests/test_pbx.c
@@ -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;
 }