From 8f8a69aa0aef26da487296d443a572a5fa107ee9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= Date: Tue, 23 Nov 2021 15:35:39 +0100 Subject: [PATCH] Fix handling of mismatched responses past timeout When a UDP dispatch receives a mismatched response, it checks whether there is still enough time to wait for the correct one to arrive before the timeout fires. If there is not, the result code is set to ISC_R_TIMEDOUT, but it is not subsequently used anywhere as 'response' is set to NULL a few lines earlier. This results in the higher-level read callback (resquery_response() in case of resolver code) not being called. However, shortly afterwards, a few levels up the call chain, isc__nm_udp_read_cb() calls isc__nmsocket_timer_stop() on the dispatch socket, effectively disabling read timeout handling for that socket. Combined with the fact that reading is not restarted in such a case (e.g. by calling dispatch_getnext() from udp_recv()), this leads to the higher-level query structure remaining referenced indefinitely because the dispatch socket it uses will neither be read from nor closed due to a timeout. This in turn causes fetch contexts to linger around indefinitely, which in turn i.a. prevents certain cache nodes (those containing rdatasets used by fetch contexts, like fctx->nameservers) from being cleaned. Fix by making sure the higher-level callback does get invoked with the ISC_R_TIMEDOUT result code when udp_recv() determines there is no more time left to receive the correct UDP response before the timeout fires. This allows the higher-level callback to clean things up, preventing the reference leak described above. --- lib/dns/dispatch.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/lib/dns/dispatch.c b/lib/dns/dispatch.c index cc7273bcb4..e52d7970a7 100644 --- a/lib/dns/dispatch.c +++ b/lib/dns/dispatch.c @@ -564,16 +564,25 @@ udp_recv(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, next: /* - * This is the wrong response. Don't call the caller back - * but keep listening. + * This is the wrong response. Check whether there is still enough + * time to wait for the correct one to arrive before the timeout fires. */ - response = NULL; - timeout = resp->timeout - dispentry_runtime(resp); if (timeout <= 0) { + /* + * The time window for receiving the correct response is + * already closed, libuv has just not processed the socket + * timer yet. Invoke the read callback, indicating a timeout. + */ eresult = ISC_R_TIMEDOUT; goto done; } + + /* + * Do not invoke the read callback just yet and instead wait for the + * proper response to arrive until the original timeout fires. + */ + response = NULL; dispatch_getnext(disp, resp, resp->timeout - dispentry_runtime(resp)); done: