From: Evan Hunt Date: Thu, 20 Sep 2018 06:38:23 +0000 (-0700) Subject: copy ns_hook objects before adding them to a hook table X-Git-Tag: v9.13.5~7^2~11 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=0b988256c79e754cf3130cf78efb52aafdaa1878;p=thirdparty%2Fbind9.git 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 ff4218a18b9..0fb64b97765 100644 --- 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 0f0abbf4ea5..bc43f86abb5 100644 --- 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, ©->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 e0418051ada..e61d15db725 100644 --- 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 e9b703805d5..6df013d2953 100644 --- 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 2532042e82f..9f24f6c4eb6 100644 --- 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 4b7002ae9cf..e88a4bd4f12 100644 --- 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() */