From f831e758d19282e692dfcdd1ccdf1f172b4d3c03 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. --- 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 166358b348..35d29eefd8 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3080,10 +3080,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); @@ -3426,10 +3425,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 2771a5b64da24ddbb385bfb4a5b6e3f5d2363a19 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. --- bin/dig/dighost.c | 1 + 1 file changed, 1 insertion(+) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 35d29eefd8..721c68f20f 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3844,6 +3844,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 5b2b3e589c2ffa63af1437f0c887304716bc4d96 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. --- 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 721c68f20f..57ccf415e4 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3135,7 +3135,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 ef9bd8533a5b699894d8917b51a6ad1fc3420481 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] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 4c3248b148..1dcc51a3be 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]