From 14afc8425b7e034b714ebff896a9b5aab9b0fe12 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Thu, 5 Oct 2017 09:42:31 +0200 Subject: [PATCH] [master] Prevent dig INSIST failures and hangs in some failure modes 4756. [bug] Interrupting dig could lead to an INSIST failure after certain errors were encountered while querying a host whose name resolved to more than one address. Change 4537 increased the odds of triggering this issue by causing dig to hang indefinitely when certain error paths were evaluated. dig now also retries TCP queries (once) if the server gracefully closes the connection before sending a response. [RT #42832, #45159] --- CHANGES | 9 +++++++++ bin/dig/dighost.c | 27 ++++++++++++++++++++++++--- bin/dig/include/dig/dig.h | 1 + 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/CHANGES b/CHANGES index 91dfc2c672..8752fd9a00 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,12 @@ +4756. [bug] Interrupting dig could lead to an INSIST failure after + certain errors were encountered while querying a host + whose name resolved to more than one address. Change + 4537 increased the odds of triggering this issue by + causing dig to hang indefinitely when certain error + paths were evaluated. dig now also retries TCP queries + (once) if the server gracefully closes the connection + before sending a response. [RT #42832, #45159] + 4755. [cleanup] Silence unnecessary log message when NZF file doesn't exist. [RT #46186] diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 3dd23d8710..e0058e5a8d 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -646,6 +646,7 @@ make_empty_lookup(void) { looknew->mapped = ISC_TRUE; looknew->dscp = -1; looknew->rrcomments = 0; + looknew->eoferr = 0; dns_fixedname_init(&looknew->fdomain); ISC_LINK_INIT(looknew, link); ISC_LIST_INIT(looknew->q); @@ -737,6 +738,7 @@ clone_lookup(dig_lookup_t *lookold, isc_boolean_t servers) { looknew->done_as_is = lookold->done_as_is; looknew->dscp = lookold->dscp; looknew->rrcomments = lookold->rrcomments; + looknew->eoferr = lookold->eoferr; if (lookold->ecs_addr != NULL) { size_t len = sizeof(isc_sockaddr_t); @@ -2775,9 +2777,12 @@ send_udp(dig_query_t *query) { next = ISC_LIST_NEXT(query, link); l = query->lookup; clear_query(query); - if (next == NULL) + if (next == NULL) { printf(";; No acceptable nameservers\n"); - check_next_lookup(l); + check_next_lookup(l); + } else { + send_udp(next); + } return; } @@ -2909,7 +2914,7 @@ tcp_length_done(isc_task_t *task, isc_event_t *event) { isc_buffer_t *b = NULL; isc_result_t result; dig_query_t *query = NULL; - dig_lookup_t *l; + dig_lookup_t *l, *n; isc_uint16_t length; REQUIRE(event->ev_type == ISC_SOCKEVENT_RECVDONE); @@ -2944,13 +2949,20 @@ tcp_length_done(isc_task_t *task, isc_event_t *event) { sizeof(sockstr)); printf(";; communications error to %s: %s\n", sockstr, isc_result_totext(sevent->result)); + if (keep != NULL) + isc_socket_detach(&keep); l = query->lookup; isc_socket_detach(&query->sock); sockcount--; debug("sockcount=%d", sockcount); INSIST(sockcount >= 0); + if (sevent->result == ISC_R_EOF && l->eoferr == 0U) { + n = requeue_lookup(l, ISC_TRUE); + n->eoferr++; + } isc_event_free(&event); clear_query(query); + cancel_lookup(l); check_next_lookup(l); UNLOCK_LOOKUP; return; @@ -3443,13 +3455,20 @@ recv_done(isc_task_t *task, isc_event_t *event) { } else { printf(";; communications error: %s\n", isc_result_totext(sevent->result)); + if (keep != NULL) + isc_socket_detach(&keep); isc_socket_detach(&query->sock); sockcount--; debug("sockcount=%d", sockcount); INSIST(sockcount >= 0); } + if (sevent->result == ISC_R_EOF && l->eoferr == 0U) { + n = requeue_lookup(l, ISC_TRUE); + n->eoferr++; + } isc_event_free(&event); clear_query(query); + cancel_lookup(l); check_next_lookup(l); UNLOCK_LOOKUP; return; @@ -3511,6 +3530,7 @@ recv_done(isc_task_t *task, isc_event_t *event) { if (fail) { isc_event_free(&event); clear_query(query); + cancel_lookup(l); check_next_lookup(l); UNLOCK_LOOKUP; return; @@ -3614,6 +3634,7 @@ recv_done(isc_task_t *task, isc_event_t *event) { if (l->tcp_mode) { isc_event_free(&event); clear_query(query); + cancel_lookup(l); check_next_lookup(l); UNLOCK_LOOKUP; return; diff --git a/bin/dig/include/dig/dig.h b/bin/dig/include/dig/dig.h index 664b309e47..e34ee02f48 100644 --- a/bin/dig/include/dig/dig.h +++ b/bin/dig/include/dig/dig.h @@ -168,6 +168,7 @@ struct dig_lookup { unsigned int ednsflags; dns_opcode_t opcode; int rrcomments; + unsigned int eoferr; }; /*% The dig_query structure */