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().

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index b340c59e3a098afa32f3deb9077d267479e36c5b..c38561fa2185b692a2a81add3675c85e53c3fd0a 100644 (file)
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -212,6 +212,7 @@ struct query_ctx {
 
        ns_client_t *client;        /* client object */
        bool         detach_client; /* client needs detaching */
+       bool         async;         /* asynchronous hook running */
 
        dns_fetchresponse_t *fresp; /* recursion response */
 

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 8382b8ede3b44fce579af9aac6b48553ff81c96b..35193b9c1718aa979b6f32295679b5f7bd9b2f18 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -5070,6 +5070,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);
+       }
 }
 
 /*%
@@ -6728,6 +6731,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
@@ -11372,10 +11378,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.
         */
@@ -11513,6 +11515,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;
 }