From: Ondřej Surý <ondrej@sury.org>
Date: Fri, 27 Jul 2018 12:41:12 +0000 (+0200)
Subject: Properly call isc_refcount_destroy() immediately after isc_refcount_decrement() to... 
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=19dd3df13e1b269fd2aab4b1e447d2e5281e3fb0;p=thirdparty%2Fbind9.git

Properly call isc_refcount_destroy() immediately after isc_refcount_decrement() to limit the impact of memory ordering
---

diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c
index 892cd7062be..a64c3d76e44 100644
--- a/bin/tests/system/dyndb/driver/db.c
+++ b/bin/tests/system/dyndb/driver/db.c
@@ -90,8 +90,10 @@ detach(dns_db_t **dbp) {
 	REQUIRE(VALID_SAMPLEDB(sampledb));
 
 	isc_refcount_decrement(&sampledb->refs, &refs);
-	if (refs == 0)
+	if (refs == 0) {
+		isc_refcount_destroy(&sampledb->refs);
 		free_sampledb(sampledb);
+	}
 	*dbp = NULL;
 }
 
diff --git a/lib/dns/acl.c b/lib/dns/acl.c
index 64dde801019..44f42a415f5 100644
--- a/lib/dns/acl.c
+++ b/lib/dns/acl.c
@@ -472,7 +472,6 @@ destroy(dns_acl_t *dacl) {
 		isc_mem_free(dacl->mctx, dacl->name);
 	if (dacl->iptable != NULL)
 		dns_iptable_detach(&dacl->iptable);
-	isc_refcount_destroy(&dacl->refcount);
 	dacl->magic = 0;
 	isc_mem_putanddetach(&dacl->mctx, dacl, sizeof(*dacl));
 }
@@ -485,8 +484,10 @@ dns_acl_detach(dns_acl_t **aclp) {
 	REQUIRE(DNS_ACL_VALID(acl));
 
 	isc_refcount_decrement(&acl->refcount, &refs);
-	if (refs == 0)
+	if (refs == 0) {
+		isc_refcount_destroy(&acl->refcount);
 		destroy(acl);
+	}
 	*aclp = NULL;
 }
 
diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index 724420c1205..51b1ac90604 100644
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -249,10 +249,10 @@ dns_catz_entry_detach(dns_catz_zone_t *zone, dns_catz_entry_t **entryp) {
 
 	isc_refcount_decrement(&entry->refs, &refs);
 	if (refs == 0) {
+		isc_refcount_destroy(&entry->refs);
 		dns_catz_options_free(&entry->opts, mctx);
 		if (dns_name_dynamic(&entry->name))
 			dns_name_free(&entry->name, mctx);
-		isc_refcount_destroy(&entry->refs);
 		isc_mem_put(mctx, entry, sizeof(dns_catz_entry_t));
 	}
 }
@@ -751,6 +751,7 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 	*zonep = NULL;
 	isc_refcount_decrement(&zone->refs, &refs);
 	if (refs == 0) {
+		isc_refcount_destroy(&zone->refs);
 		if (zone->entries != NULL) {
 			result = isc_ht_iter_create(zone->entries, &iter);
 			INSIST(result == ISC_R_SUCCESS);
@@ -772,7 +773,6 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 		}
 		mctx = zone->catzs->mctx;
 		isc_timer_detach(&zone->updatetimer);
-		isc_refcount_destroy(&zone->refs);
 		if (zone->db_registered == ISC_TRUE) {
 			result = dns_db_updatenotify_unregister(zone->db,
 						    dns_catz_dbupdate_callback,
@@ -811,6 +811,7 @@ dns_catz_catzs_detach(dns_catz_zones_t ** catzsp) {
 	isc_refcount_decrement(&catzs->refs, &refs);
 
 	if (refs == 0) {
+		isc_refcount_destroy(&catzs->refs);
 		DESTROYLOCK(&catzs->lock);
 		if (catzs->zones != NULL) {
 			result = isc_ht_iter_create(catzs->zones, &iter);
@@ -827,7 +828,6 @@ dns_catz_catzs_detach(dns_catz_zones_t ** catzsp) {
 			INSIST(isc_ht_count(catzs->zones) == 0);
 			isc_ht_destroy(&catzs->zones);
 		}
-		isc_refcount_destroy(&catzs->refs);
 		isc_task_destroy(&catzs->updater);
 		isc_mem_putanddetach(&catzs->mctx, catzs, sizeof(*catzs));
 	}
diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c
index d4a974a5c5e..078ab84e08c 100644
--- a/lib/dns/dnstap.c
+++ b/lib/dns/dnstap.c
@@ -586,8 +586,10 @@ dns_dt_detach(dns_dtenv_t **envp) {
 	*envp = NULL;
 
 	isc_refcount_decrement(&env->refcount, &refs);
-	if (refs == 0)
+	if (refs == 0) {
+		isc_refcount_destroy(&env->refcount);
 		destroy(env);
+	}
 }
 
 static isc_result_t
diff --git a/lib/dns/iptable.c b/lib/dns/iptable.c
index ec0c2229d03..13d4451290a 100644
--- a/lib/dns/iptable.c
+++ b/lib/dns/iptable.c
@@ -149,8 +149,10 @@ dns_iptable_detach(dns_iptable_t **tabp) {
 	unsigned int refs;
 	REQUIRE(DNS_IPTABLE_VALID(tab));
 	isc_refcount_decrement(&tab->refcount, &refs);
-	if (refs == 0)
+	if (refs == 0) {
+		isc_refcount_destroy(&tab->refcount);
 		destroy_iptable(tab);
+	}
 	*tabp = NULL;
 }
 
@@ -164,7 +166,6 @@ destroy_iptable(dns_iptable_t *dtab) {
 		dtab->radix = NULL;
 	}
 
-	isc_refcount_destroy(&dtab->refcount);
 	dtab->magic = 0;
 	isc_mem_putanddetach(&dtab->mctx, dtab, sizeof(*dtab));
 }
diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c
index 4323ab235f2..209eaa0ed83 100644
--- a/lib/dns/keytable.c
+++ b/lib/dns/keytable.c
@@ -137,9 +137,8 @@ dns_keytable_detach(dns_keytable_t **keytablep) {
 
 	isc_refcount_decrement(&keytable->references, &refs);
 	if (refs == 0) {
-		INSIST(isc_refcount_current(&keytable->active_nodes) == 0);
-		isc_refcount_destroy(&keytable->active_nodes);
 		isc_refcount_destroy(&keytable->references);
+		isc_refcount_destroy(&keytable->active_nodes);
 		dns_rbt_destroy(&keytable->table);
 		isc_rwlock_destroy(&keytable->rwlock);
 		keytable->magic = 0;
@@ -817,9 +816,10 @@ dns_keynode_detach(isc_mem_t *mctx, dns_keynode_t **keynode) {
 	REQUIRE(VALID_KEYNODE(node));
 	isc_refcount_decrement(&node->refcount, &refs);
 	if (refs == 0) {
-		if (node->key != NULL)
-			dst_key_free(&node->key);
 		isc_refcount_destroy(&node->refcount);
+		if (node->key != NULL) {
+			dst_key_free(&node->key);
+		}
 		isc_mem_put(mctx, node, sizeof(dns_keynode_t));
 	}
 	*keynode = NULL;
diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 4a4d078425a..8ab42527f55 100644
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -71,6 +71,7 @@ nta_detach(isc_mem_t *mctx, dns_nta_t **ntap) {
 	*ntap = NULL;
 	isc_refcount_decrement(&nta->refcount, &refs);
 	if (refs == 0) {
+		isc_refcount_destroy(&nta->refcount);
 		nta->magic = 0;
 		if (nta->timer != NULL) {
 			(void) isc_timer_reset(nta->timer,
@@ -78,7 +79,6 @@ nta_detach(isc_mem_t *mctx, dns_nta_t **ntap) {
 					       NULL, NULL, ISC_TRUE);
 			isc_timer_detach(&nta->timer);
 		}
-		isc_refcount_destroy(&nta->refcount);
 		if (dns_rdataset_isassociated(&nta->rdataset))
 			dns_rdataset_disassociate(&nta->rdataset);
 		if (dns_rdataset_isassociated(&nta->sigrdataset))
diff --git a/lib/dns/order.c b/lib/dns/order.c
index ccf61fcabc2..4549959104d 100644
--- a/lib/dns/order.c
+++ b/lib/dns/order.c
@@ -140,15 +140,14 @@ dns_order_detach(dns_order_t **orderp) {
 	order = *orderp;
 	REQUIRE(DNS_ORDER_VALID(order));
 	isc_refcount_decrement(&order->references, &references);
-	*orderp = NULL;
-	if (references != 0)
-		return;
-
-	order->magic = 0;
-	while ((ent = ISC_LIST_HEAD(order->ents)) != NULL) {
-		ISC_LIST_UNLINK(order->ents, ent, link);
-		isc_mem_put(order->mctx, ent, sizeof(*ent));
+	if (references == 0) {
+		isc_refcount_destroy(&order->references);
+		order->magic = 0;
+		while ((ent = ISC_LIST_HEAD(order->ents)) != NULL) {
+			ISC_LIST_UNLINK(order->ents, ent, link);
+			isc_mem_put(order->mctx, ent, sizeof(*ent));
+		}
+		isc_mem_putanddetach(&order->mctx, order, sizeof(*order));
 	}
-	isc_refcount_destroy(&order->references);
-	isc_mem_putanddetach(&order->mctx, order, sizeof(*order));
+	*orderp = NULL;
 }
diff --git a/lib/dns/portlist.c b/lib/dns/portlist.c
index c90b89bd4bb..5e31b1a5839 100644
--- a/lib/dns/portlist.c
+++ b/lib/dns/portlist.c
@@ -241,8 +241,8 @@ dns_portlist_detach(dns_portlist_t **portlistp) {
 	*portlistp = NULL;
 	isc_refcount_decrement(&portlist->refcount, &count);
 	if (count == 0) {
-		portlist->magic = 0;
 		isc_refcount_destroy(&portlist->refcount);
+		portlist->magic = 0;
 		if (portlist->list != NULL)
 			isc_mem_put(portlist->mctx, portlist->list,
 				    portlist->allocated *
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 96d7fb03199..b223c624008 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -1018,10 +1018,9 @@ free_rbtdb(dns_rbtdb_t *rbtdb, isc_boolean_t log, isc_event_t *event) {
 
 		isc_refcount_decrement(&rbtdb->current_version->references,
 				       &refs);
-		INSIST(refs == 0);
+		isc_refcount_destroy(&rbtdb->current_version->references);
 		UNLINK(rbtdb->open_versions, rbtdb->current_version, link);
 		isc_rwlock_destroy(&rbtdb->current_version->glue_rwlock);
-		isc_refcount_destroy(&rbtdb->current_version->references);
 		isc_rwlock_destroy(&rbtdb->current_version->rwlock);
 		isc_mem_put(rbtdb->common.mctx, rbtdb->current_version,
 			    sizeof(rbtdb_version_t));
@@ -1235,8 +1234,9 @@ detach(dns_db_t **dbp) {
 
 	isc_refcount_decrement(&rbtdb->references, &refs);
 
-	if (refs == 0)
+	if (refs == 0) {
 		maybe_free_rbtdb(rbtdb);
+	}
 
 	*dbp = NULL;
 }
diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index 851ecbc84ca..dd8b8ba629c 100644
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -2114,6 +2114,7 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) {
 	if (refs != 0) {
 		return;
 	}
+	isc_refcount_destroy(&rpzs->refs);
 
 	/*
 	 * Forget the last of view's rpz machinery after the last reference.
@@ -2140,7 +2141,6 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) {
 	}
 	DESTROYLOCK(&rpzs->maint_lock);
 	isc_rwlock_destroy(&rpzs->search_lock);
-	isc_refcount_destroy(&rpzs->refs);
 	isc_task_destroy(&rpzs->updater);
 	isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs));
 }
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index af3b2eb3c26..33be942acfe 100644
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -725,7 +725,6 @@ tsigkey_free(dns_tsigkey_t *key) {
 		dns_name_free(key->creator, key->mctx);
 		isc_mem_put(key->mctx, key->creator, sizeof(dns_name_t));
 	}
-	isc_refcount_destroy(&key->refs);
 	isc_mem_putanddetach(&key->mctx, key, sizeof(dns_tsigkey_t));
 }
 
@@ -740,8 +739,10 @@ dns_tsigkey_detach(dns_tsigkey_t **keyp) {
 	key = *keyp;
 	isc_refcount_decrement(&key->refs, &refs);
 
-	if (refs == 0)
+	if (refs == 0) {
+		isc_refcount_destroy(&key->refs);
 		tsigkey_free(key);
+	}
 
 	*keyp = NULL;
 }
diff --git a/lib/dns/view.c b/lib/dns/view.c
index 5b3e3608b98..4a3622ad550 100644
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -540,7 +540,6 @@ destroy(dns_view_t *view) {
 		dns_badcache_destroy(&view->failcache);
 	DESTROYLOCK(&view->new_zone_lock);
 	DESTROYLOCK(&view->lock);
-	isc_refcount_destroy(&view->references);
 	isc_mem_free(view->mctx, view->nta_file);
 	isc_mem_free(view->mctx, view->name);
 	isc_mem_putanddetach(&view->mctx, view, sizeof(*view));
@@ -588,6 +587,8 @@ view_flushanddetach(dns_view_t **viewp, isc_boolean_t flush) {
 	if (refs == 0) {
 		dns_zone_t *mkzone = NULL, *rdzone = NULL;
 
+		isc_refcount_destroy(&view->references);
+
 		LOCK(&view->lock);
 		if (!RESSHUTDOWN(view))
 			dns_resolver_shutdown(view->resolver);
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 98147483d32..418983b57d9 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -1253,7 +1253,6 @@ zone_free(dns_zone_t *zone) {
 	/* last stuff */
 	ZONEDB_DESTROYLOCK(&zone->dblock);
 	DESTROYLOCK(&zone->lock);
-	isc_refcount_destroy(&zone->erefs);
 	zone->magic = 0;
 	mctx = zone->mctx;
 	isc_mem_put(mctx, zone, sizeof(*zone));
@@ -5223,6 +5222,8 @@ dns_zone_detach(dns_zone_t **zonep) {
 	isc_refcount_decrement(&zone->erefs, &refs);
 
 	if (refs == 0) {
+		isc_refcount_destroy(&zone->erefs);
+
 		LOCK_ZONE(zone);
 		INSIST(zone != zone->raw);
 		/*
diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c
index dc161ca958b..09d35a709de 100644
--- a/lib/isccfg/aclconf.c
+++ b/lib/isccfg/aclconf.c
@@ -77,6 +77,7 @@ cfg_aclconfctx_detach(cfg_aclconfctx_t **actxp) {
 
 	isc_refcount_decrement(&actx->references, &refs);
 	if (refs == 0) {
+		isc_refcount_destroy(&actx->references);
 		for (dacl = ISC_LIST_HEAD(actx->named_acl_cache);
 		     dacl != NULL;
 		     dacl = next)
diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c
index db1ec2e6bf9..96c80b34886 100644
--- a/lib/isccfg/parser.c
+++ b/lib/isccfg/parser.c
@@ -680,6 +680,7 @@ cfg_parser_destroy(cfg_parser_t **pctxp) {
 
 	isc_refcount_decrement(&pctx->references, &refs);
 	if (refs == 0) {
+		isc_refcount_destroy(&pctx->references);
 		isc_lex_destroy(&pctx->lexer);
 		/*
 		 * Cleaning up open_files does not
@@ -3147,8 +3148,8 @@ cfg_obj_destroy(cfg_parser_t *pctx, cfg_obj_t **objp) {
 
 	isc_refcount_decrement(&obj->references, &refs);
 	if (refs == 0) {
-		obj->type->rep->free(pctx, obj);
 		isc_refcount_destroy(&obj->references);
+		obj->type->rep->free(pctx, obj);
 		isc_mem_put(pctx->mctx, obj, sizeof(cfg_obj_t));
 	}
 	*objp = NULL;
diff --git a/lib/ns/server.c b/lib/ns/server.c
index d57e2cf2544..e522e6fa7ce 100644
--- a/lib/ns/server.c
+++ b/lib/ns/server.c
@@ -134,6 +134,8 @@ ns_server_detach(ns_server_t **sctxp) {
 	if (refs == 0) {
 		ns_altsecret_t *altsecret;
 
+		isc_refcount_destroy(&sctx->references);
+		
 		sctx->magic = 0;
 
 		while ((altsecret = ISC_LIST_HEAD(sctx->altsecrets)) != NULL) {