prevent a reference leak from the ns_query_done hooks

if the NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND hook is
used in a plugin and returns NS_HOOK_RETURN, some of the
cleanup in ns_query_done() can be skipped over, leading
to reference leaks that can cause named to hang on shut
down.

this has been addressed by adding more housekeeping
code after the cleanup: tag in ns_query_done().

(cherry picked from commit c2e43582678ec5d0c40e19c671d60012b36ac312)

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index c09075452686b1abf03654ccbbcdd5e9a781be45..2392baf13f13e13cd18379c97cc95f19bdc5a161 100644 (file)
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -159,6 +159,7 @@ struct query_ctx {
 
        ns_client_t *client;        /* client object */
        bool         detach_client; /* client needs detaching */
+       bool         async;         /* asynchronous hook running */
 
        dns_fetchevent_t *event; /* recursion event */
 

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 8bdb33b088079c08e41d13a19567ec9782657630..5a75601160dbc0ce97df948691f903843da203e7 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5308,6 +5308,9 @@ qctx_clean(query_ctx_t *qctx) {
        if (qctx->db != NULL && qctx->node != NULL) {
                dns_db_detachnode(qctx->db, &qctx->node);
        }
+       if (qctx->client != NULL && qctx->client->query.gluedb != NULL) {
+               dns_db_detach(&qctx->client->query.gluedb);
+       }
 }
 
 /*%
@@ -7037,6 +7040,9 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync,
                goto cleanup_and_detach_from_quota;
        }
 
+       /* Record that an asynchronous copy of the qctx has been started */
+       qctx->async = true;
+
        /*
         * Typically the runasync() function will trigger recursion, but
         * there is no need to set NS_QUERYATTR_RECURSING. The calling hook
@@ -11972,10 +11978,6 @@ ns_query_done(query_ctx_t *qctx) {
        qctx_clean(qctx);
        qctx_freedata(qctx);
 
-       if (qctx->client->query.gluedb != NULL) {
-               dns_db_detach(&qctx->client->query.gluedb);
-       }
-
        /*
         * Clear the AA bit if we're not authoritative.
         */
@@ -12112,6 +12114,17 @@ ns_query_done(query_ctx_t *qctx) {
        return qctx->result;
 
 cleanup:
+       /*
+        * We'd only get here if one of the hooks above
+        * (NS_QUERY_DONE_BEGIN or NS_QUERY_DONE_SEND) returned
+        * NS_HOOK_RETURN. Some housekeeping may be needed.
+        */
+       qctx_clean(qctx);
+       qctx_freedata(qctx);
+       if (!qctx->async) {
+               qctx->detach_client = true;
+               query_error(qctx->client, DNS_R_SERVFAIL, __LINE__);
+       }
        return result;
 }