Don't allow recursion on staleonly lookups

Fix a crash that can happen in the following scenario:

A client request is received. There is no data for it in the cache,
(not even stale data). A resolver fetch is created as part of
recursion.

Some time later, the fetch still hasn't completed, and
stale-answer-client-timeout is triggered. A staleonly lookup is
started. It will also find no data in the cache.

So 'query_lookup()' will call 'query_gotanswer()' with ISC_R_NOTFOUND,
so this will call 'query_notfound()' and this will start recursion.

We will eventually end up in 'ns_query_recurse()' and that requires
the client query fetch to be NULL:

    REQUIRE(client->query.fetch == NULL);

If the previously started fetch is still running this assertion
fails.

The crash is easily prevented by not requiring recursion for
staleonly lookups.

Also remove a redundant setting of the staleonly flag at the end of
'query_lookup_staleonly()' before destroying the query context.

Add a system test to catch this case.

diff --git a/bin/tests/system/serve-stale/tests.sh b/bin/tests/system/serve-stale/tests.sh
index a11319c8544533906ffe5e227234936c8af4e05e..4b85dd358e88221d804ed816959d3e10095c7f43 100755 (executable)
--- a/bin/tests/system/serve-stale/tests.sh
+++ b/bin/tests/system/serve-stale/tests.sh
@@ -1072,7 +1072,8 @@ echo_i "sending queries for tests $((n+1))-$((n+4))..."
 $DIG -p ${PORT} @10.53.0.3 data.example TXT > dig.out.test$((n+1)) &
 $DIG -p ${PORT} @10.53.0.3 othertype.example CAA > dig.out.test$((n+2)) &
 $DIG -p ${PORT} @10.53.0.3 nodata.example TXT > dig.out.test$((n+3)) &
-$DIG -p ${PORT} @10.53.0.3 nxdomain.example TXT > dig.out.test$((n+4))
+$DIG -p ${PORT} @10.53.0.3 nxdomain.example TXT > dig.out.test$((n+4)) &
+$DIG -p ${PORT} @10.53.0.3 notfound.example TXT > dig.out.test$((n+5))
 
 wait
 
@@ -1112,6 +1113,16 @@ grep "example\..*30.*IN.*SOA" dig.out.test$n > /dev/null || ret=1
 if [ $ret != 0 ]; then echo_i "failed"; fi
 status=$((status+ret))
 
+# The notfound.example check is different than nxdomain.example because
+# we didn't send a prime query to add notfound.example to the cache.
+n=$((n+1))
+echo_i "check notfound.example (max-stale-ttl default) ($n)"
+ret=0
+grep "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1
+grep "ANSWER: 0," dig.out.test$n > /dev/null || ret=1
+if [ $ret != 0 ]; then echo_i "failed"; fi
+status=$((status+ret))
+
 #
 # Now test server with serve-stale answers disabled.
 #

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 9d5844084f22d0849815456fd2225ebaf6be5973..8dd1a7f3611bd8d1f7bfbfd0e842da660734afeb 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -6017,13 +6017,13 @@ query_lookup_staleonly(ns_client_t *client) {
 
        qctx_init(client, NULL, client->query.qtype, &qctx);
        dns_db_attach(client->view->cachedb, &qctx.db);
+       client->query.attributes &= ~NS_QUERYATTR_RECURSIONOK;
        client->query.dboptions |= DNS_DBFIND_STALEONLY;
        (void)query_lookup(&qctx);
        if (qctx.node != NULL) {
                dns_db_detachnode(qctx.db, &qctx.node);
        }
        qctx_freedata(&qctx);
-       client->query.dboptions &= ~DNS_DBFIND_STALEONLY;
        qctx_destroy(&qctx);
 }