From 7917e1866a6d30f9b35e0fca2529af5c9f4e8e59 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 19 May 2022 20:44:32 +0000 Subject: [PATCH 1/2] Fix a crash in dig NS search mode In special NS search mode, after the initial lookup, dig starts the followup lookup with discovered NS servers in the queries list. If one of those queries then fail, dig, as usual, tries to start the next query in the list, which results in a crash, because the NS search mode is special in a way that the queries are running in parallel, so the next query is usually already started. Apply some special logic in `recv_done()` function to deal with the described situation when handling the query result for the NS search mode. Particularly, print a warning message for the failed query, and do not try to start the next query in the list. Also, set a non-zero exit code if all the queries in the followup lookup fail. (cherry picked from commit 1290863c22c298d31f054525c7cbd2d446c42bda) --- bin/dig/dighost.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ bin/dig/dighost.h | 4 ++-- 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 3bcd47e87b..d63adac344 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3860,6 +3860,51 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, goto next_lookup; } + /* + * NSSEARCH mode is special, because the queries in the followup lookup + * are independent and they are being started in parallel, so if one of + * them fails there is no need to start the next query in the lookup, + * and this failure can be treated as a soft error (with a warning + * message), because there are usually more than one NS servers in the + * lookup's queries list. However, if there was not a single successful + * query in the followup lookup, then print an error message and exit + * with a non-zero exit code. + */ + if (l->ns_search_only && !l->trace_root) { + if (eresult == ISC_R_SUCCESS) { + l->ns_search_success = true; + } else { + char sockstr[ISC_SOCKADDR_FORMATSIZE]; + isc_sockaddr_format(&query->sockaddr, sockstr, + sizeof(sockstr)); + + dighost_warning("communications error to %s: %s", + sockstr, isc_result_totext(eresult)); + + /* + * If this is not the last query, then we detach the + * query, but keep the lookup running. + */ + if (!check_if_queries_done(l, query)) { + goto detach_query; + } + + /* + * This is the last query, and if there was not a + * single successful query in the whole lookup, then + * treat the situation as an error. + */ + if (!l->ns_search_success) { + dighost_error("no NS servers could be reached"); + if (exitcode < 9) { + exitcode = 9; + } + } + + goto cancel_lookup; + } + } + if (eresult == ISC_R_TIMEDOUT) { if (l->retries > 1 && !l->tcp_mode) { dig_query_t *newq = NULL; diff --git a/bin/dig/dighost.h b/bin/dig/dighost.h index d79cc6fc1a..d7246333e0 100644 --- a/bin/dig/dighost.h +++ b/bin/dig/dighost.h @@ -114,8 +114,8 @@ struct dig_lookup { idnin, idnout, ignore, multiline, need_search, new_search, noclass, nocrypto, nottl, ns_search_only, /*%< dig +nssearch, host -C */ - nsid, /*% Name Server ID (RFC 5001) */ - onesoa, pending, /*%< Pending a successful answer */ + ns_search_success, nsid, /*% Name Server ID (RFC 5001) */ + onesoa, pending, /*%< Pending a successful answer */ print_unknown_format, qr, raflag, recurse, section_additional, section_answer, section_authority, section_question, seenbadcookie, sendcookie, servfail_stops, From cb25d5b80da06757199794977ed91cbfa465f0f4 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 19 May 2022 21:05:16 +0000 Subject: [PATCH 2/2] Add CHANGES note for [GL #3207] (cherry picked from commit 0450c9bd32846f58bb4f53e81ccc961d9f79c891) --- CHANGES | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGES b/CHANGES index c60eec3ba3..c138c88ee2 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5907. [bug] Fix a crash in dig NS search mode when one of the NS + server queries fail. [GL #3207] + 5905. [bug] When the TCP connection would be closed/reset between the connect/accept and the read, the uv_read_start() return value would be unexpected and cause an assertion