From 317e36d47e1c5bc60783716a678255b60379590e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
Date: Fri, 19 Jul 2019 12:54:23 +0200
Subject: [PATCH 1/2] Restore locking in dns_resolver_shutdown and
 dns_resolver_attach

Although the struct dns_resolver.exiting member is protected by stdatomics, we
actually need to wait for whole dns_resolver_shutdown() to finish before
destroying the resolver object.  Otherwise, there would be a data race and some
fctx objects might not be destroyed yet at the time we tear down the
dns_resolver object.
---
 lib/dns/resolver.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 878b959f80..5ef6f65bc2 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -476,15 +476,15 @@ struct dns_resolver {
 	isc_timermgr_t *		timermgr;
 	isc_taskmgr_t *			taskmgr;
 	dns_view_t *			view;
-	bool			frozen;
+	bool				frozen;
 	unsigned int			options;
 	dns_dispatchmgr_t *		dispatchmgr;
 	dns_dispatchset_t *		dispatches4;
-	bool			exclusivev4;
+	bool				exclusivev4;
 	dns_dispatchset_t *		dispatches6;
 	isc_dscp_t			querydscp4;
 	isc_dscp_t			querydscp6;
-	bool			exclusivev6;
+	bool				exclusivev6;
 	unsigned int			nbuckets;
 	fctxbucket_t *			buckets;
 	zonebucket_t *			dbuckets;
@@ -524,7 +524,7 @@ struct dns_resolver {
 	unsigned int			spillat;	/* clients-per-query */
 	unsigned int			zspill;		/* fetches-per-zone */
 
-	dns_badcache_t  * 		badcache;	 /* Bad cache. */
+	dns_badcache_t  *		badcache;	 /* Bad cache. */
 
 	/* Locked by primelock. */
 	dns_fetch_t *			primefetch;
@@ -10326,6 +10326,7 @@ dns_resolver_shutdown(dns_resolver_t *res) {
 
 	RTRACE("shutdown");
 
+	LOCK(&res->lock);
 	if (atomic_compare_exchange_strong(&res->exiting, &is_false, true)) {
 		RTRACE("exiting");
 
@@ -10357,6 +10358,7 @@ dns_resolver_shutdown(dns_resolver_t *res) {
 					 NULL, true);
 		RUNTIME_CHECK(result == ISC_R_SUCCESS);
 	}
+	UNLOCK(&res->lock);
 }
 
 void
@@ -10372,8 +10374,10 @@ dns_resolver_detach(dns_resolver_t **resp) {
 	*resp = NULL;
 
 	if (isc_refcount_decrement(&res->references) == 1) {
+		LOCK(&res->lock);
 		INSIST(atomic_load_acquire(&res->exiting));
 		INSIST(res->activebuckets == 0);
+		UNLOCK(&res->lock);
 		destroy(res);
 	}
 }

From a4141fcf981018ced5a4e878e21403e350a20c78 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
Date: Mon, 22 Jul 2019 08:45:23 -0400
Subject: [PATCH 2/2] Restore more locking in the lib/dns/resolver.c code

1. Restore locking in the fctx_decreference() code, because the insides of the
   function needs to be protected when fctx->references drops to 0.

2. Restore locking in the dns_resolver_attach() code, because two variables are
   accessed at the same time and there's slight chance of data race.
---
 lib/dns/resolver.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 5ef6f65bc2..c46abb5db3 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -9384,7 +9384,9 @@ rctx_resend(respctx_t *rctx, dns_adbaddrinfo_t *addrinfo) {
 	}
 
 	fctx_done(fctx, result, __LINE__);
+	LOCK(&fctx->res->buckets[fctx->bucketnum].lock);
 	bucket_empty = fctx_decreference(fctx);
+	UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock);
 	if (bucket_empty) {
 		empty_bucket(fctx->res);
 	}
@@ -10278,8 +10280,10 @@ dns_resolver_attach(dns_resolver_t *source, dns_resolver_t **targetp) {
 
 	RRTRACE(source, "attach");
 
+	LOCK(&source->lock);
 	REQUIRE(!atomic_load_acquire(&source->exiting));
 	isc_refcount_increment(&source->references);
+	UNLOCK(&source->lock);
 
 	*targetp = source;
 }
@@ -10736,9 +10740,9 @@ dns_resolver_destroyfetch(dns_fetch_t **fetchp) {
 			RUNTIME_CHECK(event->fetch != fetch);
 		}
 	}
-	UNLOCK(&res->buckets[bucketnum].lock);
 
 	bucket_empty = fctx_decreference(fctx);
+	UNLOCK(&res->buckets[bucketnum].lock);
 
 	isc_mem_putanddetach(&fetch->mctx, fetch, sizeof(*fetch));
 	*fetchp = NULL;