parent
a7c33f0154
commit
294087c110
@ -0,0 +1,37 @@
|
||||
From e73ecbf039c3b2cd33dd2926691a8a346c9ca574 Mon Sep 17 00:00:00 2001
|
||||
From: Matthijs Mekking <matthijs@isc.org>
|
||||
Date: Tue, 2 Aug 2022 14:21:40 +0200
|
||||
Subject: [PATCH 1/3] Don't enable serve-stale on duplicate queries
|
||||
|
||||
When checking if we should enable serve-stale, add an early out case
|
||||
when the result is an error signalling a duplicate query or a query
|
||||
that would be dropped.
|
||||
|
||||
(cherry picked from commit 059a4c2f4d9d3cff371842f43208d021509314fa)
|
||||
(cherry picked from commit dd7dde5743715dc0dec2defbb92b1a8637977bf9)
|
||||
---
|
||||
lib/ns/query.c | 8 ++++++++
|
||||
1 file changed, 8 insertions(+)
|
||||
|
||||
diff --git a/lib/ns/query.c b/lib/ns/query.c
|
||||
index 4f61374e2c..c169e22bf4 100644
|
||||
--- a/lib/ns/query.c
|
||||
+++ b/lib/ns/query.c
|
||||
@@ -7205,6 +7205,14 @@ query_usestale(query_ctx_t *qctx, isc_result_t result) {
|
||||
return (false);
|
||||
}
|
||||
|
||||
+ if (result == DNS_R_DUPLICATE || result == DNS_R_DROP) {
|
||||
+ /*
|
||||
+ * Don't enable serve-stale if the result signals a duplicate
|
||||
+ * query or query that is being dropped.
|
||||
+ */
|
||||
+ return (false);
|
||||
+ }
|
||||
+
|
||||
qctx_clean(qctx);
|
||||
qctx_freedata(qctx);
|
||||
|
||||
--
|
||||
2.40.1
|
||||
|
@ -0,0 +1,72 @@
|
||||
From 589c06568e3036bfe713d42b53c8e88005ce17e4 Mon Sep 17 00:00:00 2001
|
||||
From: Evan Hunt <each@isc.org>
|
||||
Date: Thu, 25 May 2023 23:53:50 -0700
|
||||
Subject: [PATCH 2/3] Stale answer lookups could loop when over recursion quota
|
||||
|
||||
When a query was aborted because of the recursion quota being exceeded,
|
||||
but triggered a stale answer response and a stale data refresh query,
|
||||
it could cause named to loop back where we are iterating and following
|
||||
a delegation. Having no good answer in cache, we would fall back to
|
||||
using serve-stale again, use the stale data, try to refresh the RRset,
|
||||
and loop back again, without ever terminating until crashing due to
|
||||
stack overflow.
|
||||
|
||||
This happens because in the functions 'query_notfound()' and
|
||||
'query_delegation_recurse()', we check whether we can fall back to
|
||||
serving stale data. We shouldn't do so if we are already refreshing
|
||||
an RRset due to having prioritized stale data in cache.
|
||||
|
||||
In other words, we need to add an extra check to 'query_usestale()' to
|
||||
disallow serving stale data if we are currently refreshing a stale
|
||||
RRset.
|
||||
|
||||
As an additional mitigation to prevent looping, we now use the result
|
||||
code ISC_R_ALREADYRUNNING rather than ISC_R_FAILURE when a recursion
|
||||
loop is encountered, and we check for that condition in
|
||||
'query_usestale()' as well.
|
||||
|
||||
(cherry picked from commit 0101e28f91fb36b6a16a0049d3b3e2b7846f23f0)
|
||||
---
|
||||
lib/ns/query.c | 17 ++++++++++++++---
|
||||
1 file changed, 14 insertions(+), 3 deletions(-)
|
||||
|
||||
diff --git a/lib/ns/query.c b/lib/ns/query.c
|
||||
index c169e22bf4..1eb662ea4d 100644
|
||||
--- a/lib/ns/query.c
|
||||
+++ b/lib/ns/query.c
|
||||
@@ -6229,7 +6229,7 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
|
||||
if (recparam_match(&client->query.recparam, qtype, qname, qdomain)) {
|
||||
ns_client_log(client, NS_LOGCATEGORY_CLIENT, NS_LOGMODULE_QUERY,
|
||||
ISC_LOG_INFO, "recursion loop detected");
|
||||
- return (ISC_R_FAILURE);
|
||||
+ return (ISC_R_ALREADYRUNNING);
|
||||
}
|
||||
|
||||
recparam_update(&client->query.recparam, qtype, qname, qdomain);
|
||||
@@ -7205,10 +7205,21 @@ query_usestale(query_ctx_t *qctx, isc_result_t result) {
|
||||
return (false);
|
||||
}
|
||||
|
||||
- if (result == DNS_R_DUPLICATE || result == DNS_R_DROP) {
|
||||
+ if (qctx->refresh_rrset) {
|
||||
+ /*
|
||||
+ * This is a refreshing query, we have already prioritized
|
||||
+ * stale data, so don't enable serve-stale again.
|
||||
+ */
|
||||
+ return (false);
|
||||
+ }
|
||||
+
|
||||
+ if (result == DNS_R_DUPLICATE || result == DNS_R_DROP ||
|
||||
+ result == ISC_R_ALREADYRUNNING)
|
||||
+ {
|
||||
/*
|
||||
* Don't enable serve-stale if the result signals a duplicate
|
||||
- * query or query that is being dropped.
|
||||
+ * query or a query that is being dropped or can't proceed
|
||||
+ * because of a recursion loop.
|
||||
*/
|
||||
return (false);
|
||||
}
|
||||
--
|
||||
2.40.1
|
||||
|
@ -0,0 +1,60 @@
|
||||
From c20e9d30bae58d3120aa7c6a0e5dcae0e7e93dbd Mon Sep 17 00:00:00 2001
|
||||
From: Matthijs Mekking <matthijs@isc.org>
|
||||
Date: Thu, 1 Jun 2023 10:03:48 +0200
|
||||
Subject: [PATCH 3/3] Fix serve-stale hang at shutdown
|
||||
|
||||
The 'refresh_rrset' variable is used to determine if we can detach from
|
||||
the client. This can cause a hang on shutdown. To fix this, move setting
|
||||
of the 'nodetach' variable up to where 'refresh_rrset' is set (in
|
||||
query_lookup(), and thus not in ns_query_done()), and set it to false
|
||||
when actually refreshing the RRset, so that when this lookup is
|
||||
completed, the client will be detached.
|
||||
|
||||
(cherry picked from commit c003c5bc3c68f3e513654b6689e1f60280d14844)
|
||||
---
|
||||
lib/ns/query.c | 13 ++++++++-----
|
||||
1 file changed, 8 insertions(+), 5 deletions(-)
|
||||
|
||||
diff --git a/lib/ns/query.c b/lib/ns/query.c
|
||||
index 1eb662ea4d..4fe3e30f45 100644
|
||||
--- a/lib/ns/query.c
|
||||
+++ b/lib/ns/query.c
|
||||
@@ -5644,6 +5644,7 @@ query_refresh_rrset(query_ctx_t *orig_qctx) {
|
||||
qctx.client->query.dboptions &= ~(DNS_DBFIND_STALETIMEOUT |
|
||||
DNS_DBFIND_STALEOK |
|
||||
DNS_DBFIND_STALEENABLED);
|
||||
+ qctx.client->nodetach = false;
|
||||
|
||||
/*
|
||||
* We'll need some resources...
|
||||
@@ -5868,7 +5869,14 @@ query_lookup(query_ctx_t *qctx) {
|
||||
"%s stale answer used, an attempt to "
|
||||
"refresh the RRset will still be made",
|
||||
namebuf);
|
||||
+
|
||||
qctx->refresh_rrset = STALE(qctx->rdataset);
|
||||
+
|
||||
+ /*
|
||||
+ * If we are refreshing the RRSet, we must not
|
||||
+ * detach from the client in query_send().
|
||||
+ */
|
||||
+ qctx->client->nodetach = qctx->refresh_rrset;
|
||||
}
|
||||
} else {
|
||||
/*
|
||||
@@ -11469,12 +11477,7 @@ ns_query_done(query_ctx_t *qctx) {
|
||||
/*
|
||||
* Client may have been detached after query_send(), so
|
||||
* we test and store the flag state here, for safety.
|
||||
- * If we are refreshing the RRSet, we must not detach from the client
|
||||
- * in the query_send(), so we need to override the flag.
|
||||
*/
|
||||
- if (qctx->refresh_rrset) {
|
||||
- qctx->client->nodetach = true;
|
||||
- }
|
||||
nodetach = qctx->client->nodetach;
|
||||
query_send(qctx->client);
|
||||
|
||||
--
|
||||
2.40.1
|
||||
|
Loading…
Reference in new issue