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 907770e225a570534e61f3aaddfb722266e8aae8..341014cea1ca82ecb6d050496a79e9ab0c3ee09a 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 dc30d9fe7cf3ad57958b1e588499dec4319316fd..6e8e9e4f7d77fb2e726fb4a04350ff882ceedeb4 100644 (file)
--- a/channels/chan_iax2.c
+++ b/channels/chan_iax2.c
@@ -14636,7 +14636,6 @@ static void cleanup_thread_list(void *head)
 
 static int __unload_module(void)
 {
-       struct ast_context *con;
        int x;
 
        network_change_stasis_unsubscribe();
@@ -14713,9 +14712,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 b1fbd6e9b6f72051755a536dd9430053c806e0cd..5be0200ed6a202d2f4dc5ea6e78ee562ff306085 100644 (file)
--- a/channels/chan_sip.c
+++ b/channels/chan_sip.c
@@ -19750,8 +19750,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");
        }
 }
 
@@ -34540,7 +34539,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;
 
@@ -34718,10 +34716,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 be9f9b67ef5a3981681ec48c06e43a50c1421bfa..3d2d2e8d1fb11a785c5a7ae4fb33255ce298a553 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");
        }
 }
 
@@ -8710,7 +8709,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);
@@ -8771,9 +8769,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 795af05842aa4e118994b8cee4ac43428254e68d..59974587875d61bf828e5a4e67b1e680f18f45a5 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 2be01b28e888ad9e2e99e245de7fafc9be82ccbe..7686b4b5d0b49f57e421552ca2044f71f7ecd41c 100644 (file)
--- a/main/pbx.c
+++ b/main/pbx.c
@@ -8742,9 +8742,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 */
@@ -8768,11 +8768,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 */
@@ -9671,18 +9673,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;
 }
 
 /*
@@ -10890,6 +10898,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 b0364250471f5a7ded7d11f8c83ec8ef53044241..c4a0e6c286b3854c55da0194e5b9dd286d24917b 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 604d8e3011f77c18435d8d375342cc5a4d61eae3..05a3550e0aa51d18307bb8bc1806c9b5c6a0d920 100644 (file)
--- a/tests/test_gosub.c
+++ b/tests/test_gosub.c
@@ -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;
 }

diff --git a/tests/test_pbx.c b/tests/test_pbx.c
index 5e2e232edf8e5a5f7393b4b012a3eef5293b6944..bb5d8e8715a89fd9255af2289df7b60cec8c79d0 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;
 }