From 4a311b9bb4788fe4f7820379956b4697eb819024 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. (cherry picked from commit ac889684c78bc54fc537a0d97b12ddd13c0b0267) --- lib/dns/resolver.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 4f2e363b99..e136a74266 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2298,6 +2298,12 @@ cleanup_dispatch: } cleanup_query: + LOCK(&res->buckets[fctx->bucketnum].lock); + if (ISC_LINK_LINKED(query, link)) { + atomic_fetch_sub_release(&fctx->nqueries, 1); + ISC_LIST_UNLINK(fctx->queries, query, link); + } + UNLOCK(&res->buckets[fctx->bucketnum].lock); query->magic = 0; dns_message_detach(&query->rmessage); From a83a58467da71cb7607bb5d78978573f88f77527 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. (cherry picked from commit e4569373ca05e2e1c1c04d2b81c1b592acf927e5) --- 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 e136a74266..6e030b17b8 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1422,11 +1422,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 64feeba60f3b058229594eeec1553b04ea94822d 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. (cherry picked from commit 5da79e2be01700c1dd01e5a3f69617129036bb02) --- 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 6e030b17b8..5d26ebd0ff 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -2279,7 +2279,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 */ @@ -2290,6 +2290,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 192373a26e21121ec1d7c9019c5c57b81c007f25 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] (cherry picked from commit 6f50972e5f2a6fcd7465d943d6c43b6cb52fee13) --- CHANGES | 4 ++++ doc/notes/notes-current.rst | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 69bd3f5079..59297237ed 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 0f953cc038..abbae59fcf 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -46,3 +46,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`