From e0be643f5087fe3e84ef9991e90aa5abf2f18129 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 14 Jun 2022 13:13:32 +0200 Subject: [PATCH] Make async hooks code use the 'recursions' array Async hooks are the last feature using the client->fetchhandle and client->query.fetch pointers. Update ns_query_hookasync() and query_hookresume() so that they use a dedicated slot in the 'recursions' array. Note that async hooks are still not expected to initiate recursion if one was already started by a prior ns_query_recurse() call, so the REQUIRE assertion in ns_query_hookasync() needs to check the RECTYPE_NORMAL slot rather than the RECTYPE_HOOK one. --- lib/ns/include/ns/query.h | 5 +++++ lib/ns/query.c | 20 +++++++++----------- tests/ns/query_test.c | 8 ++++---- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h index 67b008c7fa..321e83eeca 100644 --- a/lib/ns/include/ns/query.h +++ b/lib/ns/include/ns/query.h @@ -46,6 +46,7 @@ typedef enum { RECTYPE_NORMAL, RECTYPE_PREFETCH, RECTYPE_RPZ, + RECTYPE_HOOK, RECTYPE_COUNT, } ns_query_rectype_t; @@ -59,6 +60,8 @@ typedef enum { ((client)->query.recursions[RECTYPE_PREFETCH].handle) #define HANDLE_RECTYPE_RPZ(client) \ ((client)->query.recursions[RECTYPE_RPZ].handle) +#define HANDLE_RECTYPE_HOOK(client) \ + ((client)->query.recursions[RECTYPE_HOOK].handle) /*% * Helper macros for accessing dns_fetch_t pointers for a specific recursion a @@ -70,6 +73,8 @@ typedef enum { ((client)->query.recursions[RECTYPE_PREFETCH].fetch) #define FETCH_RECTYPE_RPZ(client) \ ((client)->query.recursions[RECTYPE_RPZ].fetch) +#define FETCH_RECTYPE_HOOK(client) \ + ((client)->query.recursions[RECTYPE_HOOK].fetch) /*% * nameserver recursion parameters, to uniquely identify a recursion diff --git a/lib/ns/query.c b/lib/ns/query.c index e2e55a243f..17bfcd3ee7 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6693,12 +6693,11 @@ query_hookresume(isc_task_t *task, isc_event_t *event) { UNLOCK(&client->manager->reclock); /* - * This event is running under a client task, so it's safe to detach - * the fetch handle. And it should be done before resuming query - * processing below, since that may trigger another recursion or - * asynchronous hook event. + * The fetch handle should be detached before resuming query processing + * below, since that may trigger another recursion or asynchronous hook + * event. */ - isc_nmhandle_detach(&client->fetchhandle); + isc_nmhandle_detach(&HANDLE_RECTYPE_HOOK(client)); client->state = NS_CLIENTSTATE_WORKING; @@ -6814,7 +6813,7 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync, REQUIRE(NS_CLIENT_VALID(client)); REQUIRE(client->query.hookactx == NULL); - REQUIRE(client->query.fetch == NULL); + REQUIRE(FETCH_RECTYPE_NORMAL(client) == NULL); result = check_recursionquota(client); if (result != ISC_R_SUCCESS) { @@ -6837,12 +6836,11 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync, * attribute won't be checked anywhere. * * Hook-based asynchronous processing cannot coincide with normal - * recursion, so we can safely use fetchhandle here. Unlike in - * ns_query_recurse(), we attach to the handle only if 'runasync' - * succeeds. It should be safe since we're either in the client - * task or pausing it. + * recursion. Unlike in ns_query_recurse(), we attach to the handle + * only if 'runasync' succeeds. It should be safe since we're either in + * the client task or pausing it. */ - isc_nmhandle_attach(client->handle, &client->fetchhandle); + isc_nmhandle_attach(client->handle, &HANDLE_RECTYPE_HOOK(client)); return (ISC_R_SUCCESS); cleanup: diff --git a/tests/ns/query_test.c b/tests/ns/query_test.c index dda2c07150..5636801b14 100644 --- a/tests/ns/query_test.c +++ b/tests/ns/query_test.c @@ -709,11 +709,11 @@ hook_async_common(void *arg, void *data, isc_result_t *resultp, } } else { /* - * Resume from the completion of async event. - * fetchhandle should have been detached so that we can start - * another async event or DNS recursive resolution. + * Resume from the completion of async event. The fetch handle + * should have been detached so that we can start another async + * event or DNS recursive resolution. */ - INSIST(qctx->client->fetchhandle == NULL); + INSIST(HANDLE_RECTYPE_HOOK(qctx->client) == NULL); asdata->async = false; switch (hookpoint) { case NS_QUERY_GOT_ANSWER_BEGIN: