From 761cedc2b4e93fcd474da550ab9cb04f69642fef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@sury.org>
Date: Mon, 20 May 2019 16:55:42 +0200
Subject: [PATCH] Don't use rwlocks for reference counting

    WARNING: ThreadSanitizer: data race
    Write of size 8 at 0x000000000001 by thread T1 (mutexes: write M1):
    #0 memset <null>
    #1 mem_put lib/isc/mem.c:819
    #2 isc___mem_free lib/isc/mem.c:1662
    #3 isc__mem_free lib/isc/mem.c:3078
    #4 isc___mem_putanddetach lib/isc/mem.c:1221
    #5 isc__mem_putanddetach lib/isc/mem.c:3033
    #6 destroyring lib/dns/tsig.c:494
    #7 dns_tsigkeyring_dumpanddetach lib/dns/tsig.c:665
    #8 destroy lib/dns/view.c:392
    #9 dns_view_weakdetach lib/dns/view.c:704
    #10 zone_free lib/dns/zone.c:1152
    #11 zone_shutdown lib/dns/zone.c:13123
    #12 dispatch lib/isc/task.c:1157
    #13 run lib/isc/task.c:1331
    #14 <null> <null>

    Previous atomic read of size 8 at 0x000000000001 by thread T2:
    #0 __tsan_atomic64_load <null>
    #1 isc_rwlock_unlock lib/isc/rwlock.c:612
    #2 dns_tsigkeyring_dumpanddetach lib/dns/tsig.c:632
    #3 destroy lib/dns/view.c:392
    #4 dns_view_weakdetach lib/dns/view.c:704
    #5 zone_free lib/dns/zone.c:1149
    #6 zone_shutdown lib/dns/zone.c:13123
    #7 dispatch lib/isc/task.c:1157
    #8 run lib/isc/task.c:1331
    #9 <null> <null>
---
 lib/dns/include/dns/tsig.h |  2 +-
 lib/dns/tsig.c             | 38 +++++++++++++++-----------------------
 2 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/lib/dns/include/dns/tsig.h b/lib/dns/include/dns/tsig.h
index 4c05be49e7..2fa1ab1a97 100644
--- a/lib/dns/include/dns/tsig.h
+++ b/lib/dns/include/dns/tsig.h
@@ -69,7 +69,7 @@ struct dns_tsig_keyring {
 	unsigned int generated;
 	unsigned int maxgenerated;
 	ISC_LIST(dns_tsigkey_t) lru;
-	unsigned int references;
+	isc_refcount_t references;
 };
 
 struct dns_tsigkey {
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index 4d4b68f2fa..540b03f895 100644
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -368,8 +368,9 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
 	if (ring != NULL)
 		refs++;
 	ret = isc_refcount_init(&tkey->refs, refs);
-	if (ret != ISC_R_SUCCESS)
+	if (ret != ISC_R_SUCCESS) {
 		goto cleanup_creator;
+	}
 
 	tkey->generated = generated;
 	tkey->inception = inception;
@@ -407,8 +408,9 @@ dns_tsigkey_createfromkey(dns_name_t *name, dns_name_t *algorithm,
 
  cleanup_refs:
 	tkey->magic = 0;
-	while (refs-- > 0)
+	while (refs-- > 0) {
 		isc_refcount_decrement(&tkey->refs, NULL);
+	}
 	isc_refcount_destroy(&tkey->refs);
  cleanup_creator:
 	if (tkey->key != NULL)
@@ -625,14 +627,10 @@ dns_tsigkeyring_dumpanddetach(dns_tsig_keyring_t **ringp, FILE *fp) {
 	ring = *ringp;
 	*ringp = NULL;
 
-	RWLOCK(&ring->lock, isc_rwlocktype_write);
-	INSIST(ring->references > 0);
-	ring->references--;
-	references = ring->references;
-	RWUNLOCK(&ring->lock, isc_rwlocktype_write);
-
-	if (references != 0)
+	isc_refcount_decrement(&ring->references, &references);
+	if (references > 0) {
 		return (DNS_R_CONTINUE);
+	}
 
 	isc_stdtime_get(&now);
 	dns_name_init(&foundname, NULL);
@@ -1932,7 +1930,7 @@ dns_tsigkeyring_create(isc_mem_t *mctx, dns_tsig_keyring_t **ringp) {
 	ring->maxgenerated = DNS_TSIG_MAXGENERATEDKEYS;
 	ISC_LIST_INIT(ring->lru);
 	isc_mem_attach(mctx, &ring->mctx);
-	ring->references = 1;
+	isc_refcount_init(&ring->references, 1);
 
 	*ringp = ring;
 	return (ISC_R_SUCCESS);
@@ -1945,8 +1943,9 @@ dns_tsigkeyring_add(dns_tsig_keyring_t *ring, dns_name_t *name,
 	isc_result_t result;
 
 	result = keyring_add(ring, name, tkey);
-	if (result == ISC_R_SUCCESS)
+	if (result == ISC_R_SUCCESS) {
 		isc_refcount_increment(&tkey->refs, NULL);
+	}
 
 	return (result);
 }
@@ -1957,12 +1956,9 @@ dns_tsigkeyring_attach(dns_tsig_keyring_t *source, dns_tsig_keyring_t **target)
 	REQUIRE(source != NULL);
 	REQUIRE(target != NULL && *target == NULL);
 
-	RWLOCK(&source->lock, isc_rwlocktype_write);
-	INSIST(source->references > 0);
-	source->references++;
-	INSIST(source->references > 0);
+	isc_refcount_increment(&source->references, NULL);
+
 	*target = source;
-	RWUNLOCK(&source->lock, isc_rwlocktype_write);
 }
 
 void
@@ -1976,14 +1972,10 @@ dns_tsigkeyring_detach(dns_tsig_keyring_t **ringp) {
 	ring = *ringp;
 	*ringp = NULL;
 
-	RWLOCK(&ring->lock, isc_rwlocktype_write);
-	INSIST(ring->references > 0);
-	ring->references--;
-	references = ring->references;
-	RWUNLOCK(&ring->lock, isc_rwlocktype_write);
-
-	if (references == 0)
+	isc_refcount_decrement(&ring->references, &references);
+	if (references == 0) {
 		destroyring(ring);
+	}
 }
 
 void