From ac889684c78bc54fc537a0d97b12ddd13c0b0267 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 21 Oct 2022 08:08:37 +0000 Subject: [PATCH 1/4] Unlink the query under cleanup_query In the cleanup code of fctx_query() function there is a code path where 'query' is linked to 'fctx' and it is being destroyed. Make sure that 'query' is unlinked before destroying it. --- lib/dns/resolver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 5d1e7291ed..3f3c5f96c5 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2335,6 +2335,12 @@ cleanup_dispatch: } cleanup_query: + LOCK(&fctx->bucket->lock); + if (ISC_LINK_LINKED(query, link)) { + atomic_fetch_sub_release(&fctx->nqueries, 1); + ISC_LIST_UNLINK(fctx->queries, query, link); + } + UNLOCK(&fctx->bucket->lock); query->magic = 0; dns_message_detach(&query->rmessage); From e4569373ca05e2e1c1c04d2b81c1b592acf927e5 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 21 Oct 2022 08:08:47 +0000 Subject: [PATCH 2/4] Always call dns_adb_endudpfetch() in fctx_cancelquery() for UDP queries It is currently possible that dns_adb_endudpfetch() is not called in fctx_cancelquery() for a UDP query, which results in quotas not being adjusted back. Always call dns_adb_endudpfetch() for UDP queries. --- lib/dns/resolver.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 3f3c5f96c5..70fd0c204e 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1427,11 +1427,11 @@ fctx_cancelquery(resquery_t **queryp, isc_time_t *finish, bool no_response, } dns_adb_adjustsrtt(fctx->adb, query->addrinfo, rtt, factor); + } - if ((query->options & DNS_FETCHOPT_TCP) == 0) { - /* Inform the ADB that we're ending a UDP fetch */ - dns_adb_endudpfetch(fctx->adb, query->addrinfo); - } + if ((query->options & DNS_FETCHOPT_TCP) == 0) { + /* Inform the ADB that we're ending a UDP fetch */ + dns_adb_endudpfetch(fctx->adb, query->addrinfo); } /* From 5da79e2be01700c1dd01e5a3f69617129036bb02 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 21 Oct 2022 08:08:55 +0000 Subject: [PATCH 3/4] Call dns_adb_endudpfetch() on error path, if required For UDP queries, after calling dns_adb_beginudpfetch() in fctx_query(), make sure that dns_adb_endudpfetch() is also called on error path, in order to adjust the quota back. --- lib/dns/resolver.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 70fd0c204e..cc552ea33a 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2316,7 +2316,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, resquery_senddone, resquery_response, query, &query->id, &query->dispentry); if (result != ISC_R_SUCCESS) { - goto cleanup_dispatch; + goto cleanup_udpfetch; } /* Connect the socket */ @@ -2327,6 +2327,14 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, return (result); +cleanup_udpfetch: + if (!RESQUERY_CANCELED(query)) { + if ((query->options & DNS_FETCHOPT_TCP) == 0) { + /* Inform the ADB that we're ending a UDP fetch */ + dns_adb_endudpfetch(fctx->adb, addrinfo); + } + } + cleanup_dispatch: fctx_detach(&query->fctx); From 6f50972e5f2a6fcd7465d943d6c43b6cb52fee13 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Fri, 21 Oct 2022 08:09:01 +0000 Subject: [PATCH 4/4] Add CHANGES and release notes for [GL #3598] --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 944496db2d..c0e6b5b0a8 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6001. [bug] Always call dns_adb_endudpfetch() after calling + dns_adb_beginudpfetch() for UDP queries in resolver.c, + in order to adjust back the quota. [GL #3598] + 6000. [bug] Fix a startup issue on Solaris systems with many (reportedly > 510) CPUs. Thanks to Stacey Marshall from Oracle for deep investigation of the problem. [GL #3563] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 77092bd75a..e4729b039d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -43,3 +43,7 @@ Bug Fixes - BIND would fail to start on Solaris-based systems with hundreds of CPUs. This has been fixed. ISC would like to thank Stacey Marshall from Oracle for bringing this problem to our attention. :gl:`#3563` + +- In certain resolution scenarios quotas could be erroneously reached for + servers, including the configured forwarders, resulting in SERVFAIL answers + sent to the clients. This has been fixed. :gl:`#3598`