From 8e57030f69d92cae238ab4397a4b006028aa8984 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 31 Mar 2022 14:33:49 +0000 Subject: [PATCH 1/4] When using +qr in dig print the data of the current query In `send_udp()` and `launch_next_query()` functions, when calling `dighost_printmessage()` to print detailed information about the sent query, dig always prints the data of the first query in the lookup's queries list. The first query in the list can be already finished, having its handles freed, and accessing this information results in assertion failure. Print the current query's information instead. (cherry picked from commit f831e758d19282e692dfcdd1ccdf1f172b4d3c03) --- bin/dig/dighost.c | 11 +++++------ bin/tests/system/digdelv/tests.sh | 9 +++++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 627ad61b5b..18f6f54f63 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2901,10 +2901,9 @@ send_udp(dig_query_t *query) { debug("sendcount=%" PRIuFAST32, isc_refcount_current(&sendcount)); /* XXX qrflag, print_query, etc... */ - if (!ISC_LIST_EMPTY(query->lookup->q) && query->lookup->qr) { + if (query->lookup->qr) { extrabytes = 0; - dighost_printmessage(ISC_LIST_HEAD(query->lookup->q), - &query->lookup->renderbuf, + dighost_printmessage(query, &query->lookup->renderbuf, query->lookup->sendmsg, true); if (query->lookup->stats) { print_query_size(query); @@ -3247,10 +3246,10 @@ launch_next_query(dig_query_t *query) { isc_refcount_current(&sendcount)); /* XXX qrflag, print_query, etc... */ - if (!ISC_LIST_EMPTY(l->q) && l->qr) { + if (l->qr) { extrabytes = 0; - dighost_printmessage(ISC_LIST_HEAD(l->q), &l->renderbuf, - l->sendmsg, true); + dighost_printmessage(query, &l->renderbuf, l->sendmsg, + true); if (l->stats) { print_query_size(query); } diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh index 6c28ac7bca..767e33f8e6 100644 --- a/bin/tests/system/digdelv/tests.sh +++ b/bin/tests/system/digdelv/tests.sh @@ -1081,6 +1081,15 @@ if [ -x "$DIG" ] ; then grep -F ";; No acceptable nameservers" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) + + # See [GL #3244] for more information + n=$((n+1)) + echo_i "check that dig handles printing query information with +qr and +y when multiple queries are involved (including a failed query) ($n)" + ret=0 + dig_with_opts +timeout=1 +qr +y @127.0.0.1 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 + grep -F "IN A 10.0.0.1" dig.out.test$n > /dev/null || ret=1 + if [ $ret -ne 0 ]; then echo_i "failed"; fi + status=$((status+ret)) else echo_i "$DIG is needed, so skipping these dig tests" fi From 927f00e15df323a58e5145affdad972bdde03889 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 29 Mar 2022 13:01:24 +0000 Subject: [PATCH 2/4] Add a missing clear_current_lookup() call in recv_done() The error code path handling the `ISC_R_CANCELED` code lacks a `clear_current_lookup()` call, without which dig hangs indefinitely when handling the error. Add the missing call to account for all references of the lookup so it can be destroyed. (cherry picked from commit 2771a5b64da24ddbb385bfb4a5b6e3f5d2363a19) --- bin/dig/dighost.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 18f6f54f63..5753a5c424 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3654,6 +3654,7 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, } query_detach(&query); lookup_detach(&l); + clear_current_lookup(); UNLOCK_LOOKUP; return; } From bf9bec6f91ea8eb0390a7ec7e37736c3f6e8540f Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 31 Mar 2022 22:00:21 +0000 Subject: [PATCH 3/4] Fix using unset pointer when printing a debug message in dighost.c The used `query->handle` is always `NULL` at this point. Change the code to use `handle` instead. (cherry picked from commit 5b2b3e589c2ffa63af1437f0c887304716bc4d96) --- bin/dig/dighost.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 5753a5c424..939fd6690a 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2956,7 +2956,7 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { query_attach(query, &readquery); debug("recving with lookup=%p, query=%p, handle=%p", query->lookup, - query, query->handle); + query, handle); query->handle = handle; isc_nmhandle_attach(handle, &query->readhandle); From 3914408950a3269279d522ac61b6d2a264e77759 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 31 Mar 2022 23:17:56 +0000 Subject: [PATCH 4/4] Add CHANGES note for [GL #3244] (cherry picked from commit ef9bd8533a5b699894d8917b51a6ad1fc3420481) --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index f3b521e85a..f8d008375f 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +5853. [bug] When using both the `+qr` and `+y` options `dig` could + crash if the connection to the first server was not + successful. [GL #3244] + 5852. [func] Add new "load-balance-socket" option to enable/disable load balancing of sockets. [GL #3249]