Add some DBC checks in dighost; fix race between clear_query and send_done in dighost

diff --git a/CHANGES b/CHANGES
index d44ef6711ec6225c89d855768be5b118b940b838..8cb9ea1b505198b1363554b7af0aa6777d86263e 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+5082.  [bug]           Fixed a race that could cause a crash in
+                       dig/host/nslookup. [GL #650]
+
 5081.  [func]          Use per-worker queues in task manager, make task
                        runners CPU-affine. [GL #659]
 

diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c
index 4daddbda7e3a9e61693e4482a730e158373058d2..416a3eaa5cfb3258efdaeed21c809ce0e7ae4c56 100644 (file)
--- a/bin/dig/dighost.c
+++ b/bin/dig/dighost.c
@@ -1519,17 +1519,21 @@ clear_query(dig_query_t *query) {
 
        debug("clear_query(%p)", query);
 
-       if (query->timer != NULL)
+       if (query->timer != NULL) {
                isc_timer_detach(&query->timer);
+       }
        lookup = query->lookup;
 
-       if (lookup->current_query == query)
+       if (lookup->current_query == query) {
                lookup->current_query = NULL;
+       }
 
-       if (ISC_LINK_LINKED(query, link))
+       if (ISC_LINK_LINKED(query, link)) {
                ISC_LIST_UNLINK(lookup->q, query, link);
-       if (ISC_LINK_LINKED(query, clink))
+       }
+       if (ISC_LINK_LINKED(query, clink)) {
                ISC_LIST_UNLINK(lookup->connecting, query, clink);
+       }
        INSIST(query->recvspace != NULL);
 
        if (query->sock != NULL) {
@@ -1541,10 +1545,13 @@ clear_query(dig_query_t *query) {
        isc_mempool_put(commctx, query->tmpsendspace);
        isc_buffer_invalidate(&query->recvbuf);
        isc_buffer_invalidate(&query->lengthbuf);
-       if (query->waiting_senddone)
+
+       if (query->waiting_senddone) {
                query->pending_free = true;
-       else
+       } else {
+               query->magic = 0;
                isc_mem_free(mctx, query);
+       }
 }
 
 /*%
@@ -2452,13 +2459,14 @@ setup_lookup(dig_lookup_t *lookup) {
 
        for (serv = ISC_LIST_HEAD(lookup->my_server_list);
             serv != NULL;
-            serv = ISC_LIST_NEXT(serv, link)) {
+            serv = ISC_LIST_NEXT(serv, link))
+       {
                query = isc_mem_allocate(mctx, sizeof(dig_query_t));
-               if (query == NULL)
+               if (query == NULL) {
                        fatal("memory allocation failure in %s:%d",
                              __FILE__, __LINE__);
-               debug("create query %p linked to lookup %p",
-                      query, lookup);
+               }
+               debug("create query %p linked to lookup %p", query, lookup);
                query->lookup = lookup;
                query->timer = NULL;
                query->waiting_connect = false;
@@ -2482,16 +2490,21 @@ setup_lookup(dig_lookup_t *lookup) {
                query->sock = NULL;
                query->recvspace = isc_mempool_get(commctx);
                query->tmpsendspace = isc_mempool_get(commctx);
-               if (query->recvspace == NULL)
+               if (query->recvspace == NULL) {
                        fatal("memory allocation failure");
+               }
 
                isc_buffer_init(&query->recvbuf, query->recvspace, COMMSIZE);
                isc_buffer_init(&query->lengthbuf, query->lengthspace, 2);
-               isc_buffer_init(&query->tmpsendbuf, query->tmpsendspace, COMMSIZE);
+               isc_buffer_init(&query->tmpsendbuf, query->tmpsendspace,
+                               COMMSIZE);
                query->sendbuf = lookup->renderbuf;
 
                ISC_LINK_INIT(query, clink);
                ISC_LINK_INIT(query, link);
+
+               query->magic = DIG_QUERY_MAGIC;
+
                ISC_LIST_ENQUEUE(lookup->q, query, link);
        }
 
@@ -2500,9 +2513,10 @@ setup_lookup(dig_lookup_t *lookup) {
                extrabytes = 0;
                dighost_printmessage(ISC_LIST_HEAD(lookup->q),
                                     lookup->sendmsg, true);
-               if (lookup->stats)
+               if (lookup->stats) {
                        printf(";; QUERY SIZE: %u\n\n",
                               isc_buffer_usedlength(&lookup->renderbuf));
+               }
        }
        return (true);
 }
@@ -2528,20 +2542,26 @@ send_done(isc_task_t *_task, isc_event_t *event) {
        INSIST(sendcount >= 0);
 
        query = event->ev_arg;
+       REQUIRE(DIG_VALID_QUERY(query));
        query->waiting_senddone = false;
        l = query->lookup;
 
-       if (l->ns_search_only && !l->trace_root && !l->tcp_mode) {
+       if (!query->pending_free && l->ns_search_only &&
+           !l->trace_root && !l->tcp_mode)
+       {
                debug("sending next, since searching");
                next = ISC_LIST_NEXT(query, link);
-               if (next != NULL)
+               if (next != NULL) {
                        send_udp(next);
+               }
        }
 
        isc_event_free(&event);
 
-       if (query->pending_free)
+       if (query->pending_free) {
+               query->magic = 0;
                isc_mem_free(mctx, query);
+       }
 
        check_if_done();
        UNLOCK_LOOKUP;
@@ -2559,6 +2579,7 @@ cancel_lookup(dig_lookup_t *lookup) {
        debug("cancel_lookup()");
        query = ISC_LIST_HEAD(lookup->q);
        while (query != NULL) {
+               REQUIRE(DIG_VALID_QUERY(query));
                next = ISC_LIST_NEXT(query, link);
                if (query->sock != NULL) {
                        isc_socket_cancel(query->sock, global_task,
@@ -2578,6 +2599,7 @@ bringup_timer(dig_query_t *query, unsigned int default_timeout) {
        dig_lookup_t *l;
        unsigned int local_timeout;
        isc_result_t result;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        debug("bringup_timer()");
        /*
@@ -2642,6 +2664,7 @@ send_tcp_connect(dig_query_t *query) {
        isc_result_t result;
        dig_query_t *next;
        dig_lookup_t *l;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        debug("send_tcp_connect(%p)", query);
 
@@ -2770,6 +2793,7 @@ send_udp(dig_query_t *query) {
        dig_query_t *next;
        isc_region_t r;
        isc_socketevent_t *sevent;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        debug("send_udp(%p)", query);
 
@@ -2871,6 +2895,7 @@ connect_timeout(isc_task_t *task, isc_event_t *event) {
 
        LOCK_LOOKUP;
        query = event->ev_arg;
+       REQUIRE(DIG_VALID_QUERY(query));
        l = query->lookup;
        isc_event_free(&event);
 
@@ -2957,6 +2982,7 @@ tcp_length_done(isc_task_t *task, isc_event_t *event) {
        LOCK_LOOKUP;
        sevent = (isc_socketevent_t *)event;
        query = event->ev_arg;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        recvcount--;
        INSIST(recvcount >= 0);
@@ -3031,6 +3057,7 @@ launch_next_query(dig_query_t *query, bool include_question) {
        isc_result_t result;
        dig_lookup_t *l;
        isc_region_t r;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        INSIST(!free_now);
 
@@ -3103,6 +3130,7 @@ connect_done(isc_task_t *task, isc_event_t *event) {
        LOCK_LOOKUP;
        sevent = (isc_socketevent_t *)event;
        query = sevent->ev_arg;
+       REQUIRE(DIG_VALID_QUERY(query));
 
        INSIST(query->waiting_connect);
 
@@ -3994,6 +4022,7 @@ do_lookup(dig_lookup_t *lookup) {
        lookup->pending = true;
        query = ISC_LIST_HEAD(lookup->q);
        if (query != NULL) {
+               REQUIRE(DIG_VALID_QUERY(query));
                if (lookup->tcp_mode)
                        send_tcp_connect(query);
                else

diff --git a/bin/dig/include/dig/dig.h b/bin/dig/include/dig/dig.h
index a224c705a311ae6d00213e42caab10905c647c06..23ad8d686dc5f4d5bef1c9b37cc329767e6cc005 100644 (file)
--- a/bin/dig/include/dig/dig.h
+++ b/bin/dig/include/dig/dig.h
@@ -26,6 +26,7 @@
 #include <isc/formatcheck.h>
 #include <isc/lang.h>
 #include <isc/list.h>
+#include <isc/magic.h>
 #include <isc/mem.h>
 #include <isc/print.h>
 #include <isc/sockaddr.h>
@@ -81,6 +82,11 @@ typedef struct dig_server dig_server_t;
 typedef ISC_LIST(dig_server_t) dig_serverlist_t;
 typedef struct dig_searchlist dig_searchlist_t;
 
+#define DIG_QUERY_MAGIC                ISC_MAGIC('D','i','g','q')
+
+#define DIG_VALID_QUERY(x)     ISC_MAGIC_VALID((x), DIG_QUERY_MAGIC)
+
+
 /*% The dig_lookup structure */
 struct dig_lookup {
        bool
@@ -184,6 +190,7 @@ struct dig_lookup {
 
 /*% The dig_query structure */
 struct dig_query {
+       unsigned int magic;
        dig_lookup_t *lookup;
        bool waiting_connect,
                pending_free,