2124. [bug] It was possible to dereference a freed fetch

                        context. [RT #16584]

diff --git a/CHANGES b/CHANGES
index 6975d4efe35cd8dcc746fd42bdaf0d5bebeea0ac..7d3f2906a1414d4b05ae7acf5c4490a455cf460b 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+2124.  [bug]           It was possible to dereference a freed fetch
+                       context. [RT #16584]
+
        --- 9.3.3-WFB-1 released ---
 
 2035.  [func]          Make falling back to TCP on UDP refresh failure

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index b9e69c8c873099febb463ac741ab0709d4d6b99b..3e0efcbc5de40af3e2e7862568b5d1f2f429e732 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -15,7 +15,7 @@
  * PERFORMANCE OF THIS SOFTWARE.
  */
 
-/* $Id: resolver.c,v 1.218.2.18.4.64 2006/08/31 03:57:11 marka Exp $ */
+/* $Id: resolver.c,v 1.218.2.18.4.64.2.1 2007/01/04 05:54:56 marka Exp $ */
 
 #include <config.h>
 
@@ -218,6 +218,11 @@ struct fetchctx {
        dns_name_t                      nsname; 
        dns_fetch_t *                   nsfetch;
        dns_rdataset_t                  nsrrset;
+
+       /*%
+        * Number of queries that reference this context.
+        */
+       unsigned int                    nqueries;
 };
 
 #define FCTX_MAGIC                     ISC_MAGIC('F', '!', '!', '!')
@@ -351,6 +356,7 @@ static isc_result_t ncache_adderesult(dns_message_t *message,
                                      dns_rdataset_t *ardataset,
                                      isc_result_t *eresultp);
 static void validated(isc_task_t *task, isc_event_t *event); 
+static void maybe_destroy(fetchctx_t *fctx);
 
 static isc_result_t
 valcreate(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, dns_name_t *name,
@@ -515,6 +521,9 @@ resquery_destroy(resquery_t **queryp) {
 
        INSIST(query->tcpsocket == NULL);
 
+       query->fctx->nqueries--;
+       if (SHUTTINGDOWN(query->fctx))
+               maybe_destroy(query->fctx);     /* Locks bucket. */
        query->magic = 0;
        isc_mem_put(query->mctx, query, sizeof(*query));
        *queryp = NULL;
@@ -1088,6 +1097,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo,
        }
 
        ISC_LIST_APPEND(fctx->queries, query, link);
+       query->fctx->nqueries++;
 
        return (ISC_R_SUCCESS);
 
@@ -1540,7 +1550,7 @@ fctx_finddone(isc_task_t *task, isc_event_t *event) {
                        want_done = ISC_TRUE;
                }
        } else if (SHUTTINGDOWN(fctx) && fctx->pending == 0 &&
-                  ISC_LIST_EMPTY(fctx->validators)) {
+                  fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators)) {
                bucketnum = fctx->bucketnum;
                LOCK(&res->buckets[bucketnum].lock);
                /*
@@ -2394,8 +2404,8 @@ fctx_destroy(fetchctx_t *fctx) {
        REQUIRE(ISC_LIST_EMPTY(fctx->finds));
        REQUIRE(ISC_LIST_EMPTY(fctx->altfinds));
        REQUIRE(fctx->pending == 0);
-       REQUIRE(ISC_LIST_EMPTY(fctx->validators));
        REQUIRE(fctx->references == 0);
+       REQUIRE(ISC_LIST_EMPTY(fctx->validators));
 
        FCTXTRACE("destroy");
 
@@ -2569,7 +2579,7 @@ fctx_doshutdown(isc_task_t *task, isc_event_t *event) {
        }
 
        if (fctx->references == 0 && fctx->pending == 0 &&
-           ISC_LIST_EMPTY(fctx->validators))
+           fctx->nqueries == 0 && ISC_LIST_EMPTY(fctx->validators))
                bucket_empty = fctx_destroy(fctx);
 
        UNLOCK(&res->buckets[bucketnum].lock);
@@ -2610,6 +2620,7 @@ fctx_start(isc_task_t *task, isc_event_t *event) {
                 * pending ADB finds and no pending validations.
                 */
                INSIST(fctx->pending == 0);
+               INSIST(fctx->nqueries == 0);
                INSIST(ISC_LIST_EMPTY(fctx->validators));
                if (fctx->references == 0) {
                        /*
@@ -2771,6 +2782,7 @@ fctx_create(dns_resolver_t *res, dns_name_t *name, dns_rdatatype_t type,
        fctx->restarts = 0;
        fctx->timeouts = 0;
        fctx->attributes = 0;
+       fctx->nqueries = 0;
 
        dns_name_init(&fctx->nsname, NULL);
        fctx->nsfetch = NULL;
@@ -3096,7 +3108,8 @@ maybe_destroy(fetchctx_t *fctx) {
 
        REQUIRE(SHUTTINGDOWN(fctx));
 
-       if (fctx->pending != 0 || !ISC_LIST_EMPTY(fctx->validators))
+       if (fctx->pending != 0 || fctx->nqueries != 0 ||
+           !ISC_LIST_EMPTY(fctx->validators))
                return;
 
        bucketnum = fctx->bucketnum;
@@ -6371,7 +6384,8 @@ dns_resolver_destroyfetch(dns_fetch_t **fetchp) {
                /*
                 * No one cares about the result of this fetch anymore.
                 */
-               if (fctx->pending == 0 && ISC_LIST_EMPTY(fctx->validators) &&
+               if (fctx->pending == 0 && fctx->nqueries == 0 &&
+                   ISC_LIST_EMPTY(fctx->validators) &&
                    SHUTTINGDOWN(fctx)) {
                        /*
                         * This fctx is already shutdown; we were just