copy ns_hook objects before adding them to a hook table

- this is necessary because adding the same hook to multiple views
  causes the ISC_LIST link value to become inconsistent; it isn't
  noticeable when only one hook action is ever registered at a
  given hook point, but it will break things when there are two.

diff --git a/bin/hooks/filter-aaaa.c b/bin/hooks/filter-aaaa.c
index ff4218a18b963f70e5fc32236f4afe6f5a1d7d0e..0fb64b977654dc12aa5a4853f4db7ec1331a964c 100644 (file)
--- a/bin/hooks/filter-aaaa.c
+++ b/bin/hooks/filter-aaaa.c
@@ -105,42 +105,42 @@ static isc_ht_t *client_ht = NULL;
  */
 static bool
 filter_qctx_initialize(void *arg, void *cbdata, isc_result_t *resp);
-static ns_hook_t filter_init = {
+static const ns_hook_t filter_init = {
        .action = filter_qctx_initialize,
        .action_data = &client_ht,
 };
 
 static bool
 filter_respond_begin(void *arg, void *cbdata, isc_result_t *resp);
-static ns_hook_t filter_respbegin = {
+static const ns_hook_t filter_respbegin = {
        .action = filter_respond_begin,
        .action_data = &client_ht,
 };
 
 static bool
 filter_respond_any_found(void *arg, void *cbdata, isc_result_t *resp);
-static ns_hook_t filter_respanyfound = {
+static const ns_hook_t filter_respanyfound = {
        .action = filter_respond_any_found,
        .action_data = &client_ht,
 };
 
 static bool
 filter_prep_response_begin(void *arg, void *cbdata, isc_result_t *resp);
-static ns_hook_t filter_prepresp = {
+static const ns_hook_t filter_prepresp = {
        .action = filter_prep_response_begin,
        .action_data = &client_ht,
 };
 
 static bool
 filter_query_done_send(void *arg, void *cbdata, isc_result_t *resp);
-static ns_hook_t filter_donesend = {
+static const ns_hook_t filter_donesend = {
        .action = filter_query_done_send,
        .action_data = &client_ht,
 };
 
 static bool
 filter_qctx_destroy(void *arg, void *cbdata, isc_result_t *resp);
-ns_hook_t filter_destroy = {
+static const ns_hook_t filter_destroy = {
        .action = filter_qctx_destroy,
        .action_data = &client_ht,
 };
@@ -292,13 +292,18 @@ hook_register(const char *parameters,
                CHECK(parse_parameters(parameters, cfg, actx, hctx));
        }
 
-       ns_hook_add(hooktable, NS_QUERY_QCTX_INITIALIZED, &filter_init);
-       ns_hook_add(hooktable, NS_QUERY_RESPOND_BEGIN, &filter_respbegin);
-       ns_hook_add(hooktable, NS_QUERY_RESPOND_ANY_FOUND,
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_QCTX_INITIALIZED,
+                   &filter_init);
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_RESPOND_BEGIN,
+                   &filter_respbegin);
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_RESPOND_ANY_FOUND,
                    &filter_respanyfound);
-       ns_hook_add(hooktable, NS_QUERY_PREP_RESPONSE_BEGIN, &filter_prepresp);
-       ns_hook_add(hooktable, NS_QUERY_DONE_SEND, &filter_donesend);
-       ns_hook_add(hooktable, NS_QUERY_QCTX_DESTROYED, &filter_destroy);
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_PREP_RESPONSE_BEGIN,
+                   &filter_prepresp);
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_DONE_SEND,
+                   &filter_donesend);
+       ns_hook_add(hooktable, hctx->mctx, NS_QUERY_QCTX_DESTROYED,
+                   &filter_destroy);
 
        CHECK(isc_mempool_create(hctx->mctx, sizeof(filter_data_t),
                                 &datapool));

diff --git a/lib/ns/hooks.c b/lib/ns/hooks.c
index 0f0abbf4ea530b2b027f056a34d36e1fb0c4fdc0..bc43f86abb5300337f50063e3051b520e33ef044 100644 (file)
--- a/lib/ns/hooks.c
+++ b/lib/ns/hooks.c
@@ -70,7 +70,7 @@ static isc_result_t
 load_symbol(void *handle, const char *modpath,
            const char *symbol_name, void **symbolp)
 {
-       void *symbol;
+       void *symbol = NULL;
 
        REQUIRE(handle != NULL);
        REQUIRE(symbolp != NULL && *symbolp == NULL);
@@ -187,7 +187,7 @@ cleanup:
 
 static void
 unload_library(ns_hook_module_t **hmodp) {
-       ns_hook_module_t *hmod;
+       ns_hook_module_t *hmod = NULL;
 
        REQUIRE(hmodp != NULL && *hmodp != NULL);
 
@@ -208,7 +208,7 @@ static isc_result_t
 load_symbol(HMODULE handle, const char *modpath,
            const char *symbol_name, void **symbolp)
 {
-       void *symbol;
+       void *symbol = NULL;
 
        REQUIRE(handle != NULL);
        REQUIRE(symbolp != NULL && *symbolp == NULL);
@@ -301,7 +301,7 @@ cleanup:
 
 static void
 unload_library(ns_hook_module_t **hmodp) {
-       ns_hook_module_t *hmod;
+       ns_hook_module_t *hmod = NULL;
 
        REQUIRE(hmodp != NULL && *hmodp != NULL);
 
@@ -371,7 +371,7 @@ cleanup:
 
 void
 ns_hookmodule_unload_all(void) {
-       ns_hook_module_t *hmod, *prev;
+       ns_hook_module_t *hmod = NULL, *prev = NULL;
 
        if (!hook_modules_initialized) {
                return;
@@ -393,7 +393,7 @@ ns_hookmodule_unload_all(void) {
 
 isc_result_t
 ns_hook_createctx(isc_mem_t *mctx, ns_hookctx_t **hctxp) {
-       ns_hookctx_t *hctx;
+       ns_hookctx_t *hctx = NULL;
 
        REQUIRE(hctxp != NULL && *hctxp == NULL);
 
@@ -411,7 +411,7 @@ ns_hook_createctx(isc_mem_t *mctx, ns_hookctx_t **hctxp) {
 
 void
 ns_hook_destroyctx(ns_hookctx_t **hctxp) {
-       ns_hookctx_t *hctx;
+       ns_hookctx_t *hctx = NULL;
 
        REQUIRE(hctxp != NULL && NS_HOOKCTX_VALID(*hctxp));
 
@@ -439,7 +439,7 @@ ns_hooktable_init(ns_hooktable_t *hooktable) {
 
 isc_result_t
 ns_hooktable_create(isc_mem_t *mctx, ns_hooktable_t **tablep) {
-       ns_hooktable_t *hooktable;
+       ns_hooktable_t *hooktable = NULL;
 
        REQUIRE(tablep != NULL && *tablep == NULL);
 
@@ -454,26 +454,50 @@ ns_hooktable_create(isc_mem_t *mctx, ns_hooktable_t **tablep) {
 
 void
 ns_hooktable_free(isc_mem_t *mctx, void **tablep) {
-       ns_hooktable_t *table;
+       ns_hooktable_t *table = NULL;
+       ns_hook_t *hook = NULL, *next = NULL;
+       int i = 0;
 
        REQUIRE(tablep != NULL && *tablep != NULL);
 
        table = *tablep;
        *tablep = NULL;
+
+       for (i = 0; i < NS_HOOKPOINTS_COUNT; i++) {
+               for (hook = ISC_LIST_HEAD((*table)[i]);
+                    hook != NULL;
+                    hook = next)
+               {
+                       next = ISC_LIST_NEXT(hook, link);
+                       ISC_LIST_UNLINK((*table)[i], hook, link);
+                       if (hook->mctx != NULL) {
+                               isc_mem_putanddetach(&hook->mctx,
+                                                    hook, sizeof(*hook));
+                       }
+               }
+       }
+
        isc_mem_put(mctx, table, sizeof(*table));
 }
 
 void
-ns_hook_add(ns_hooktable_t *hooktable, ns_hookpoint_t hookpoint,
-           ns_hook_t *hook)
+ns_hook_add(ns_hooktable_t *hooktable, isc_mem_t *mctx,
+           ns_hookpoint_t hookpoint, const ns_hook_t *hook)
 {
+       ns_hook_t *copy = NULL;
+
+       REQUIRE(hooktable != NULL);
+       REQUIRE(mctx != NULL);
        REQUIRE(hookpoint < NS_HOOKPOINTS_COUNT);
        REQUIRE(hook != NULL);
 
-       if (hooktable == NULL) {
-               hooktable = ns__hook_table;
-       }
+       copy = isc_mem_get(mctx, sizeof(*copy));
+       memset(copy, 0, sizeof(*copy));
+
+       copy->action = hook->action;
+       copy->action_data = hook->action_data;
+       isc_mem_attach(mctx, &copy->mctx);
 
-       ISC_LINK_INIT(hook, link);
-       ISC_LIST_APPEND((*hooktable)[hookpoint], hook, link);
+       ISC_LINK_INIT(copy, link);
+       ISC_LIST_APPEND((*hooktable)[hookpoint], copy, link);
 }

diff --git a/lib/ns/include/ns/hooks.h b/lib/ns/include/ns/hooks.h
index e0418051ada53789279c1a8a06965c0efa76ff4a..e61d15db7257d55ce785370dec10a19c824f4e37 100644 (file)
--- a/lib/ns/include/ns/hooks.h
+++ b/lib/ns/include/ns/hooks.h
@@ -201,6 +201,7 @@ typedef bool
 (*ns_hook_action_t)(void *arg, void *data, isc_result_t *resultp);
 
 typedef struct ns_hook {
+       isc_mem_t *mctx;
        ns_hook_action_t action;
        void *action_data;
        ISC_LINK(struct ns_hook) link;
@@ -327,17 +328,21 @@ ns_hookmodule_unload_all(void);
  */
 
 void
-ns_hook_add(ns_hooktable_t *hooktable, ns_hookpoint_t hookpoint,
-           ns_hook_t *hook);
+ns_hook_add(ns_hooktable_t *hooktable, isc_mem_t *mctx,
+           ns_hookpoint_t hookpoint, const ns_hook_t *hook);
 /*%<
- * Append hook function 'hook' to the list of hooks at 'hookpoint' in
- * 'hooktable'.
+ * Allocate (using memory context 'mctx') a copy of the 'hook' structure
+ * describing a hook callback and append it to the list of hooks at 'hookpoint'
+ * in 'hooktable'.
  *
  * Requires:
- *\li 'hook' is not NULL
+ *\li 'hooktable' is not NULL
+ *
+ *\li 'mctx' is not NULL
  *
  *\li 'hookpoint' is less than NS_QUERY_HOOKS_COUNT
  *
+ *\li 'hook' is not NULL
  */
 
 void

diff --git a/lib/ns/query.c b/lib/ns/query.c
index e9b703805d56e76347f69d3112a715244d6d7a49..6df013d29539ec1cfb12822e12e51ba7208ddeae 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -485,7 +485,7 @@ query_addwildcardproof(query_ctx_t *qctx, bool ispositive, bool nodata);
 static void
 query_addauth(query_ctx_t *qctx);
 
-/*%
+/*
  * Increment query statistics counters.
  */
 static inline void

diff --git a/lib/ns/tests/nstest.c b/lib/ns/tests/nstest.c
index 2532042e82f9ab5a7823fcce98e4a6df0e66d3bc..9f24f6c4eb656727bb7741c8c25dd48579880c1c 100644 (file)
--- a/lib/ns/tests/nstest.c
+++ b/lib/ns/tests/nstest.c
@@ -672,8 +672,8 @@ extract_qctx(void *arg, void *data, isc_result_t *resultp) {
  */
 static isc_result_t
 create_qctx_for_client(ns_client_t *client, query_ctx_t **qctxp) {
-       ns_hooktable_t *saved_hook_table, query_hooks;
-       ns_hook_t hook = {
+       ns_hooktable_t *saved_hook_table = NULL, *query_hooks = NULL;
+       const ns_hook_t hook = {
                .action = extract_qctx,
                .action_data = qctxp,
        };
@@ -690,15 +690,16 @@ create_qctx_for_client(ns_client_t *client, query_ctx_t **qctxp) {
         * set hooks.
         */
 
-       ns_hooktable_init(&query_hooks);
-       ns_hook_add(&query_hooks, NS_QUERY_SETUP, &hook);
+       ns_hooktable_create(mctx, &query_hooks);
+       ns_hook_add(query_hooks, mctx, NS_QUERY_SETUP, &hook);
 
        saved_hook_table = ns__hook_table;
-       ns__hook_table = &query_hooks;
+       ns__hook_table = query_hooks;
 
        ns_query_start(client);
 
        ns__hook_table = saved_hook_table;
+       ns_hooktable_free(mctx, (void **)&query_hooks);
 
        if (*qctxp == NULL) {
                return (ISC_R_NOMEMORY);

diff --git a/lib/ns/tests/query_test.c b/lib/ns/tests/query_test.c
index 4b7002ae9cf2e69c6b20c09d8d1033bb5a252cc1..e88a4bd4f12a5fde102f07736ae6cbbb121c8373 100644 (file)
--- a/lib/ns/tests/query_test.c
+++ b/lib/ns/tests/query_test.c
@@ -82,9 +82,10 @@ typedef struct {
  */
 static void
 run_sfcache_test(const ns__query_sfcache_test_params_t *test) {
+       ns_hooktable_t *query_hooks = NULL;
        query_ctx_t *qctx = NULL;
        isc_result_t result;
-       ns_hook_t hook = {
+       const ns_hook_t hook = {
                .action = ns_test_hook_catch_call,
        };
 
@@ -97,8 +98,9 @@ run_sfcache_test(const ns__query_sfcache_test_params_t *test) {
         * Interrupt execution if ns_query_done() is called.
         */
 
-       ns_hooktable_init(ns__hook_table);
-       ns_hook_add(ns__hook_table, NS_QUERY_DONE_BEGIN, &hook);
+       ns_hooktable_create(mctx, &query_hooks);
+       ns_hook_add(query_hooks, mctx, NS_QUERY_DONE_BEGIN, &hook);
+       ns__hook_table = query_hooks;
 
        /*
         * Construct a query context for a ./NS query with given flags.
@@ -157,6 +159,7 @@ run_sfcache_test(const ns__query_sfcache_test_params_t *test) {
         * Clean up.
         */
        ns_test_qctx_destroy(&qctx);
+       ns_hooktable_free(mctx, (void **)&query_hooks);
 }
 
 /* test ns__query_sfcache() */
@@ -279,9 +282,10 @@ typedef struct {
  */
 static void
 run_start_test(const ns__query_start_test_params_t *test) {
+       ns_hooktable_t *query_hooks = NULL;
        query_ctx_t *qctx = NULL;
        isc_result_t result;
-       ns_hook_t hook = {
+       const ns_hook_t hook = {
                .action = ns_test_hook_catch_call,
        };
 
@@ -295,10 +299,10 @@ run_start_test(const ns__query_start_test_params_t *test) {
        /*
         * Interrupt execution if query_lookup() or ns_query_done() is called.
         */
-
-       ns_hooktable_init(ns__hook_table);
-       ns_hook_add(ns__hook_table, NS_QUERY_LOOKUP_BEGIN, &hook);
-       ns_hook_add(ns__hook_table, NS_QUERY_DONE_BEGIN, &hook);
+       ns_hooktable_create(mctx, &query_hooks);
+       ns_hook_add(query_hooks, mctx, NS_QUERY_LOOKUP_BEGIN, &hook);
+       ns_hook_add(query_hooks, mctx, NS_QUERY_DONE_BEGIN, &hook);
+       ns__hook_table = query_hooks;
 
        /*
         * Construct a query context using the supplied parameters.
@@ -414,6 +418,7 @@ run_start_test(const ns__query_start_test_params_t *test) {
                ns_test_cleanup_zone();
        }
        ns_test_qctx_destroy(&qctx);
+       ns_hooktable_free(mctx, (void **)&query_hooks);
 }
 
 /* test ns__query_start() */