From 8806de3986819346c2e4ad4261246ea103eaefae Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Wed, 14 Dec 2016 15:42:43 +1100 Subject: [PATCH] 4537. [bug] Handle timouts better in dig/host/nslookup. [RT #43576] (cherry picked from commit 6089c8df71f1bf74f4def30ed5d01b6f217be028) --- CHANGES | 2 + bin/dig/dighost.c | 84 +++++++++++++++++++++++---------------- bin/dig/include/dig/dig.h | 5 ++- bin/dig/nslookup.c | 3 -- 4 files changed, 54 insertions(+), 40 deletions(-) diff --git a/CHANGES b/CHANGES index ca46cdf3f1..8e6aa86d03 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +4537. [bug] Handle timouts better in dig/host/nslookup. [RT #43576] + 4535. [bug] Address race condition in setting / testing of DNS_REQUEST_F_SENDING. [RT #43889] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 5239227340..ef94e20662 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -760,7 +760,6 @@ make_empty_lookup(void) { looknew->sendmsg = NULL; looknew->name = NULL; looknew->oname = NULL; - looknew->timer = NULL; looknew->xfr_q = NULL; looknew->current_query = NULL; looknew->doing_xfr = ISC_FALSE; @@ -1514,6 +1513,8 @@ clear_query(dig_query_t *query) { debug("clear_query(%p)", query); + if (query->timer != NULL) + isc_timer_detach(&query->timer); lookup = query->lookup; if (lookup->current_query == query) @@ -1607,8 +1608,6 @@ destroy_lookup(dig_lookup_t *lookup) { debug("freeing buffer %p", lookup->querysig); isc_buffer_free(&lookup->querysig); } - if (lookup->timer != NULL) - isc_timer_detach(&lookup->timer); if (lookup->sendspace != NULL) isc_mempool_put(commctx, lookup->sendspace); @@ -2363,6 +2362,7 @@ setup_lookup(dig_lookup_t *lookup) { debug("create query %p linked to lookup %p", query, lookup); query->lookup = lookup; + query->timer = NULL; query->waiting_connect = ISC_FALSE; query->waiting_senddone = ISC_FALSE; query->pending_free = ISC_FALSE; @@ -2372,6 +2372,7 @@ setup_lookup(dig_lookup_t *lookup) { query->second_rr_rcvd = ISC_FALSE; query->first_repeat_rcvd = ISC_FALSE; query->warn_id = ISC_TRUE; + query->timedout = ISC_FALSE; query->first_rr_serial = 0; query->second_rr_serial = 0; query->servname = serv->servername; @@ -2476,8 +2477,6 @@ cancel_lookup(dig_lookup_t *lookup) { } query = next; } - if (lookup->timer != NULL) - isc_timer_detach(&lookup->timer); lookup->pending = ISC_FALSE; lookup->retries = 0; } @@ -2495,7 +2494,7 @@ bringup_timer(dig_query_t *query, unsigned int default_timeout) { * just reset it. */ l = query->lookup; - if (ISC_LIST_NEXT(query, link) != NULL) + if (ISC_LINK_LINKED(query, link) && ISC_LIST_NEXT(query, link) != NULL) local_timeout = SERVER_TIMEOUT; else { if (timeout == 0) @@ -2505,21 +2504,21 @@ bringup_timer(dig_query_t *query, unsigned int default_timeout) { } debug("have local timeout of %d", local_timeout); isc_interval_set(&l->interval, local_timeout, 0); - if (l->timer != NULL) - isc_timer_detach(&l->timer); + if (query->timer != NULL) + isc_timer_detach(&query->timer); result = isc_timer_create(timermgr, isc_timertype_once, NULL, &l->interval, global_task, connect_timeout, - l, &l->timer); + query, &query->timer); check_result(result, "isc_timer_create"); } static void -force_timeout(dig_lookup_t *l, dig_query_t *query) { +force_timeout(dig_query_t *query) { isc_event_t *event; debug("force_timeout ()"); event = isc_event_allocate(mctx, query, ISC_TIMEREVENT_IDLE, - connect_timeout, l, + connect_timeout, query, sizeof(isc_event_t)); if (event == NULL) { fatal("isc_event_allocate: %s", @@ -2533,8 +2532,8 @@ force_timeout(dig_lookup_t *l, dig_query_t *query) { * We need to cancel the possible timeout event not to confuse * ourselves due to the duplicate events. */ - if (l->timer != NULL) - isc_timer_detach(&l->timer); + if (query->timer != NULL) + isc_timer_detach(&query->timer); } @@ -2564,7 +2563,7 @@ send_tcp_connect(dig_query_t *query) { * by triggering an immediate 'timeout' (we lie, but the effect * is the same). */ - force_timeout(l, query); + force_timeout(query); return; } @@ -2574,7 +2573,10 @@ send_tcp_connect(dig_query_t *query) { printf(";; Skipping server %s, incompatible " "address family\n", query->servname); query->waiting_connect = ISC_FALSE; - next = ISC_LIST_NEXT(query, link); + if (ISC_LINK_LINKED(query, link)) + next = ISC_LIST_NEXT(query, link); + else + next = NULL; l = query->lookup; clear_query(query); if (next == NULL) { @@ -2625,9 +2627,11 @@ send_tcp_connect(dig_query_t *query) { */ if (l->ns_search_only && !l->trace_root) { debug("sending next, since searching"); - next = ISC_LIST_NEXT(query, link); - if (ISC_LINK_LINKED(query, link)) + if (ISC_LINK_LINKED(query, link)) { + next = ISC_LIST_NEXT(query, link); ISC_LIST_DEQUEUE(l->q, query, link); + } else + next = NULL; ISC_LIST_ENQUEUE(l->connecting, query, clink); if (next != NULL) send_tcp_connect(next); @@ -2668,7 +2672,7 @@ send_udp(dig_query_t *query) { result = get_address(query->servname, port, &query->sockaddr); if (result != ISC_R_SUCCESS) { /* This servname doesn't have an address. */ - force_timeout(l, query); + force_timeout(query); return; } @@ -2723,7 +2727,7 @@ send_udp(dig_query_t *query) { static void connect_timeout(isc_task_t *task, isc_event_t *event) { dig_lookup_t *l = NULL; - dig_query_t *query = NULL, *next, *cq; + dig_query_t *query = NULL, *cq; UNUSED(task); REQUIRE(event->ev_type == ISC_TIMEREVENT_IDLE); @@ -2731,13 +2735,14 @@ connect_timeout(isc_task_t *task, isc_event_t *event) { debug("connect_timeout()"); LOCK_LOOKUP; - l = event->ev_arg; - query = l->current_query; + query = event->ev_arg; + l = query->lookup; isc_event_free(&event); INSIST(!free_now); if ((query != NULL) && (query->lookup->current_query != NULL) && + ISC_LINK_LINKED(query->lookup->current_query, link) && (ISC_LIST_NEXT(query->lookup->current_query, link) != NULL)) { debug("trying next server..."); cq = query->lookup->current_query; @@ -2747,14 +2752,17 @@ connect_timeout(isc_task_t *task, isc_event_t *event) { if (query->sock != NULL) isc_socket_cancel(query->sock, NULL, ISC_SOCKCANCEL_ALL); - next = ISC_LIST_NEXT(cq, link); - if (next != NULL) - send_tcp_connect(next); + send_tcp_connect(ISC_LIST_NEXT(cq, link)); } UNLOCK_LOOKUP; return; } + if (l->tcp_mode && query->sock != NULL) { + query->timedout = ISC_TRUE; + isc_socket_cancel(query->sock, NULL, ISC_SOCKCANCEL_ALL); + } + if (l->retries > 1) { if (!l->tcp_mode) { l->retries--; @@ -2769,9 +2777,11 @@ connect_timeout(isc_task_t *task, isc_event_t *event) { check_next_lookup(l); } } else { - fputs(l->cmdline, stdout); - printf(";; connection timed out; no servers could be " - "reached\n"); + if (!l->ns_search_only) { + fputs(l->cmdline, stdout); + printf(";; connection timed out; no servers could be " + "reached\n"); + } cancel_lookup(l); check_next_lookup(l); if (exitcode < 9) @@ -2937,6 +2947,7 @@ launch_next_query(dig_query_t *query, isc_boolean_t include_question) { */ static void connect_done(isc_task_t *task, isc_event_t *event) { + char sockstr[ISC_SOCKADDR_FORMATSIZE]; isc_socketevent_t *sevent = NULL; dig_query_t *query = NULL, *next; dig_lookup_t *l; @@ -2958,6 +2969,12 @@ connect_done(isc_task_t *task, isc_event_t *event) { if (sevent->result == ISC_R_CANCELED) { debug("in cancel handler"); + isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr)); + if (query->timedout) + printf(";; Connection to %s(%s) for %s failed: %s.\n", + sockstr, query->servname, + query->lookup->textname, + isc_result_totext(ISC_R_TIMEDOUT)); isc_socket_detach(&query->sock); INSIST(sockcount > 0); sockcount--; @@ -2971,7 +2988,6 @@ connect_done(isc_task_t *task, isc_event_t *event) { return; } if (sevent->result != ISC_R_SUCCESS) { - char sockstr[ISC_SOCKADDR_FORMATSIZE]; debug("unsuccessful connection: %s", isc_result_totext(sevent->result)); @@ -2982,8 +2998,8 @@ connect_done(isc_task_t *task, isc_event_t *event) { query->servname, query->lookup->textname, isc_result_totext(sevent->result)); isc_socket_detach(&query->sock); + INSIST(sockcount > 0); sockcount--; - INSIST(sockcount >= 0); /* XXX Clean up exitcodes */ if (exitcode < 9) exitcode = 9; @@ -3212,8 +3228,8 @@ recv_done(isc_task_t *task, isc_event_t *event) { INSIST(b == &query->recvbuf); ISC_LIST_DEQUEUE(sevent->bufferlist, &query->recvbuf, link); - if ((l->tcp_mode) && (l->timer != NULL)) - isc_timer_touch(l->timer); + if ((l->tcp_mode) && (query->timer != NULL)) + isc_timer_touch(query->timer); if ((!l->pending && !l->ns_search_only) || cancel_now) { debug("no longer pending. Got %s", isc_result_totext(sevent->result)); @@ -3498,7 +3514,7 @@ recv_done(isc_task_t *task, isc_event_t *event) { * the timeout to much longer, so brief network * outages won't cause the XFR to abort */ - if (timeout != INT_MAX && l->timer != NULL) { + if (timeout != INT_MAX && query->timer != NULL) { unsigned int local_timeout; if (timeout == 0) { @@ -3514,7 +3530,7 @@ recv_done(isc_task_t *task, isc_event_t *event) { } debug("have local timeout of %d", local_timeout); isc_interval_set(&l->interval, local_timeout, 0); - result = isc_timer_reset(l->timer, + result = isc_timer_reset(query->timer, isc_timertype_once, NULL, &l->interval, @@ -3797,8 +3813,6 @@ cancel_all(void) { } cancel_now = ISC_TRUE; if (current_lookup != NULL) { - if (current_lookup->timer != NULL) - isc_timer_detach(¤t_lookup->timer); for (q = ISC_LIST_HEAD(current_lookup->q); q != NULL; q = nq) diff --git a/bin/dig/include/dig/dig.h b/bin/dig/include/dig/dig.h index 2522d291f5..75d2395fc9 100644 --- a/bin/dig/include/dig/dig.h +++ b/bin/dig/include/dig/dig.h @@ -162,7 +162,6 @@ isc_boolean_t sigchase; isc_buffer_t renderbuf; char *sendspace; dns_name_t *name; - isc_timer_t *timer; isc_interval_t interval; dns_message_t *sendmsg; dns_name_t *oname; @@ -197,7 +196,8 @@ struct dig_query { second_rr_rcvd, first_repeat_rcvd, recv_made, - warn_id; + warn_id, + timedout; isc_uint32_t first_rr_serial; isc_uint32_t second_rr_serial; isc_uint32_t msg_count; @@ -222,6 +222,7 @@ struct dig_query { isc_time_t time_recv; isc_uint64_t byte_count; isc_buffer_t sendbuf; + isc_timer_t *timer; }; struct dig_server { diff --git a/bin/dig/nslookup.c b/bin/dig/nslookup.c index 36652e533c..95a95c190f 100644 --- a/bin/dig/nslookup.c +++ b/bin/dig/nslookup.c @@ -27,7 +27,6 @@ #include #include #include -#include #include #include #include @@ -877,8 +876,6 @@ flush_lookup_list(void) { } if (l->sendmsg != NULL) dns_message_destroy(&l->sendmsg); - if (l->timer != NULL) - isc_timer_detach(&l->timer); lp = l; l = ISC_LIST_NEXT(l, link); ISC_LIST_DEQUEUE(lookup_list, lp, link);