From 20424b3bfe8d3fae92c11a30e79aeffd26dc2891 Mon Sep 17 00:00:00 2001

From: Aram Sargsyan <aram@isc.org>

Date: Mon, 14 Nov 2022 12:18:06 +0000

Subject: [PATCH] Cancel all fetch events in dns_resolver_cancelfetch()

Although 'dns_fetch_t' fetch can have two associated events, one for

each of 'DNS_EVENT_FETCHDONE' and 'DNS_EVENT_TRYSTALE' types, the

dns_resolver_cancelfetch() function is designed in a way that it

expects only one existing event, which it must cancel, and when it

happens so that 'stale-answer-client-timeout' is enabled and there

are two events, only one of them is canceled, and it results in an

assertion in dns_resolver_destroyfetch(), when it finds a dangling

event.

Change the logic of dns_resolver_cancelfetch() function so that it

cancels both the events (if they exist), and in the right order.

(cherry picked from commit ec2098ca35039e4f81fd0aa7c525eb960b8f47bf)

---

 lib/dns/resolver.c | 53 +++++++++++++++++++++++++++++++++++-----------

 lib/ns/query.c     |  4 +++-

 2 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c

index 18585b5..7cbfbb2 100644

--- a/lib/dns/resolver.c

+++ b/lib/dns/resolver.c

@@ -11254,8 +11254,9 @@ void

 dns_resolver_cancelfetch(dns_fetch_t *fetch) {

 	fetchctx_t *fctx;

 	dns_resolver_t *res;

-	dns_fetchevent_t *event, *next_event;

-	isc_task_t *etask;

+	dns_fetchevent_t *event = NULL;

+	dns_fetchevent_t *event_trystale = NULL;

+	dns_fetchevent_t *event_fetchdone = NULL;

 	REQUIRE(DNS_FETCH_VALID(fetch));

 	fctx = fetch->private;

@@ -11267,32 +11268,60 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) {

 	LOCK(&res->buckets[fctx->bucketnum].lock);

 	/*

-	 * Find the completion event for this fetch (as opposed

+	 * Find the events for this fetch (as opposed

 	 * to those for other fetches that have joined the same

-	 * fctx) and send it with result = ISC_R_CANCELED.

+	 * fctx) and send them with result = ISC_R_CANCELED.

 	 */

-	event = NULL;

 	if (fctx->state != fetchstate_done) {

+		dns_fetchevent_t *next_event = NULL;

 		for (event = ISC_LIST_HEAD(fctx->events); event != NULL;

 		     event = next_event) {

 			next_event = ISC_LIST_NEXT(event, ev_link);

 			if (event->fetch == fetch) {

 				ISC_LIST_UNLINK(fctx->events, event, ev_link);

-				break;

+				switch (event->ev_type) {

+				case DNS_EVENT_TRYSTALE:

+					INSIST(event_trystale == NULL);

+					event_trystale = event;

+					break;

+				case DNS_EVENT_FETCHDONE:

+					INSIST(event_fetchdone == NULL);

+					event_fetchdone = event;

+					break;

+				default:

+					ISC_UNREACHABLE();

+				}

+				if (event_trystale != NULL &&

+				    event_fetchdone != NULL)

+				{

+					break;

+				}

 			}

 		}

 	}

-	if (event != NULL) {

-		etask = event->ev_sender;

-		event->ev_sender = fctx;

-		event->result = ISC_R_CANCELED;

-		isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event));

+

+	/*

+	 * The "trystale" event must be sent before the "fetchdone" event,

+	 * because the latter clears the "recursing" query attribute, which is

+	 * required by both events (handled by the same callback function).

+	 */

+	if (event_trystale != NULL) {

+		isc_task_t *etask = event_trystale->ev_sender;

+		event_trystale->ev_sender = fctx;

+		event_trystale->result = ISC_R_CANCELED;

+		isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_trystale));

 	}

+	if (event_fetchdone != NULL) {

+		isc_task_t *etask = event_fetchdone->ev_sender;

+		event_fetchdone->ev_sender = fctx;

+		event_fetchdone->result = ISC_R_CANCELED;

+		isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_fetchdone));

+ 	}

+

 	/*

 	 * The fctx continues running even if no fetches remain;

 	 * the answer is still cached.

 	 */

-

 	UNLOCK(&res->buckets[fctx->bucketnum].lock);

 }

diff --git a/lib/ns/query.c b/lib/ns/query.c

index f66bab4..4f61374 100644

--- a/lib/ns/query.c

+++ b/lib/ns/query.c

@@ -6021,7 +6021,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {

 	CTRACE(ISC_LOG_DEBUG(3), "fetch_callback");

 	if (event->ev_type == DNS_EVENT_TRYSTALE) {

-		query_lookup_stale(client);

+		if (devent->result != ISC_R_CANCELED) {

+			query_lookup_stale(client);

+		}

 		isc_event_free(ISC_EVENT_PTR(&event));

 		return;

 	}

-- 

2.39.1