From: Mark Andrews <marka@isc.org>
Date: Thu, 5 Dec 2019 02:29:45 +0000 (+1100)
Subject: Always check the return from isc_refcount_decrement.
X-Git-Tag: v9.17.4~25^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=bde5c7632ad62f5a9e7d2165695e6db2fc654e46;p=thirdparty%2Fbind9.git

Always check the return from isc_refcount_decrement.

Created isc_refcount_decrement_expect macro to test conditionally
the return value to ensure it is in expected range.  Converted
unchecked isc_refcount_decrement to use isc_refcount_decrement_expect.
Converted INSIST(isc_refcount_decrement()...) to isc_refcount_decrement_expect.
---

diff --git a/bin/named/server.c b/bin/named/server.c
index 0d9639717ff..81cf4b592f9 100644
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -9664,7 +9664,7 @@ load_zones(named_server_t *server, bool init, bool reconfig) {
 		isc_refcount_increment(&zl->refs);
 		result = dns_view_asyncload(view, reconfig, view_loaded, zl);
 		if (result != ISC_R_SUCCESS) {
-			(void)isc_refcount_decrement(&zl->refs);
+			isc_refcount_decrement1(&zl->refs);
 			goto cleanup;
 		}
 	}
diff --git a/lib/dns/cache.c b/lib/dns/cache.c
index a156f80e83a..d32e903d088 100644
--- a/lib/dns/cache.c
+++ b/lib/dns/cache.c
@@ -555,7 +555,7 @@ cache_cleaner_init(dns_cache_t *cache, isc_taskmgr_t *taskmgr,
 		result = isc_task_onshutdown(cleaner->task,
 					     cleaner_shutdown_action, cache);
 		if (result != ISC_R_SUCCESS) {
-			isc_refcount_decrement(&cleaner->cache->live_tasks);
+			isc_refcount_decrement0(&cleaner->cache->live_tasks);
 			UNEXPECTED_ERROR(__FILE__, __LINE__,
 					 "cache cleaner: "
 					 "isc_task_onshutdown() failed: %s",
@@ -1020,7 +1020,7 @@ cleaner_shutdown_action(isc_task_t *task, isc_event_t *event) {
 	/* Make sure we don't reschedule anymore. */
 	(void)isc_task_purge(task, NULL, DNS_EVENT_CACHECLEAN, NULL);
 
-	INSIST(isc_refcount_decrement(&cache->live_tasks) == 1);
+	isc_refcount_decrementz(&cache->live_tasks);
 
 	cache_free(cache);
 }
diff --git a/lib/dns/client.c b/lib/dns/client.c
index 11ecd053008..b48eb0c9bea 100644
--- a/lib/dns/client.c
+++ b/lib/dns/client.c
@@ -582,7 +582,7 @@ dns_client_createx(isc_mem_t *mctx, isc_appctx_t *actx, isc_taskmgr_t *taskmgr,
 	return (ISC_R_SUCCESS);
 
 cleanup_references:
-	isc_refcount_decrement(&client->references);
+	isc_refcount_decrementz(&client->references);
 	isc_refcount_destroy(&client->references);
 cleanup_dispatchmgr:
 	if (dispatchv4 != NULL) {
@@ -1787,7 +1787,7 @@ dns_client_startrequest(dns_client_t *client, dns_message_t *qmessage,
 		return (ISC_R_SUCCESS);
 	}
 
-	isc_refcount_decrement(&client->references);
+	isc_refcount_decrement1(&client->references);
 
 	LOCK(&client->lock);
 	ISC_LIST_UNLINK(client->reqctxs, ctx, link);
@@ -2946,7 +2946,7 @@ dns_client_startupdate(dns_client_t *client, dns_rdataclass_t rdclass,
 		return (result);
 	}
 
-	isc_refcount_decrement(&client->references);
+	isc_refcount_decrement1(&client->references);
 	*transp = NULL;
 
 fail:
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 6d785869c59..0e26425a301 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -1046,9 +1046,7 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) {
 	REQUIRE(rbtdb->future_version == NULL);
 
 	if (rbtdb->current_version != NULL) {
-		INSIST(isc_refcount_decrement(
-			       &rbtdb->current_version->references) == 1);
-
+		isc_refcount_decrementz(&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);
@@ -8703,7 +8701,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 	rbtdb->next_serial = 2;
 	rbtdb->current_version = allocate_version(mctx, 1, 1, false);
 	if (rbtdb->current_version == NULL) {
-		isc_refcount_decrement(&rbtdb->references);
+		isc_refcount_decrementz(&rbtdb->references);
 		free_rbtdb(rbtdb, false, NULL);
 		return (ISC_R_NOMEMORY);
 	}
@@ -8724,7 +8722,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 		isc_mem_put(mctx, rbtdb->current_version,
 			    sizeof(*rbtdb->current_version));
 		rbtdb->current_version = NULL;
-		isc_refcount_decrement(&rbtdb->references);
+		isc_refcount_decrementz(&rbtdb->references);
 		free_rbtdb(rbtdb, false, NULL);
 		return (result);
 	}
diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 893eb340d21..21f8d96e0e5 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -4487,7 +4487,7 @@ fctx_unlink(fetchctx_t *fctx) {
 
 	ISC_LIST_UNLINK(res->buckets[bucketnum].fctxs, fctx, link);
 
-	REQUIRE(atomic_fetch_sub_release(&res->nfctx, 1) > 0);
+	INSIST(atomic_fetch_sub_release(&res->nfctx, 1) > 0);
 
 	dec_stats(res, dns_resstatscounter_nfetch);
 
@@ -5185,7 +5185,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,
 
 	ISC_LIST_APPEND(res->buckets[bucketnum].fctxs, fctx, link);
 
-	REQUIRE(atomic_fetch_add_relaxed(&res->nfctx, 1) < UINT32_MAX);
+	INSIST(atomic_fetch_add_relaxed(&res->nfctx, 1) < UINT32_MAX);
 
 	inc_stats(res, dns_resstatscounter_nfetch);
 
diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index bdda8d266b8..a99c427ada9 100644
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -1502,9 +1502,9 @@ cleanup_task:
 	dns_rbt_destroy(&zones->rbt);
 
 cleanup_rbt:
-	isc_refcount_decrement(&zones->irefs);
+	isc_refcount_decrementz(&zones->irefs);
 	isc_refcount_destroy(&zones->irefs);
-	isc_refcount_decrement(&zones->refs);
+	isc_refcount_decrementz(&zones->refs);
 	isc_refcount_destroy(&zones->refs);
 
 	isc_mutex_destroy(&zones->maint_lock);
@@ -1587,7 +1587,7 @@ cleanup_ht:
 	isc_timer_detach(&zone->updatetimer);
 
 cleanup_timer:
-	isc_refcount_decrement(&zone->refs);
+	isc_refcount_decrementz(&zone->refs);
 	isc_refcount_destroy(&zone->refs);
 
 	isc_mem_put(rpzs->mctx, zone, sizeof(*zone));
diff --git a/lib/dns/sdlz.c b/lib/dns/sdlz.c
index bd712ab36eb..2271de67ea0 100644
--- a/lib/dns/sdlz.c
+++ b/lib/dns/sdlz.c
@@ -638,7 +638,7 @@ getnodedata(dns_db_t *db, const dns_name_t *name, bool create,
 	}
 
 	if (result != ISC_R_SUCCESS) {
-		isc_refcount_decrement(&node->references);
+		isc_refcount_decrementz(&node->references);
 		destroynode(node);
 		return (result);
 	}
@@ -650,7 +650,7 @@ getnodedata(dns_db_t *db, const dns_name_t *name, bool create,
 				      sdlz->dbdata, node);
 		MAYBE_UNLOCK(sdlz->dlzimp);
 		if (result != ISC_R_SUCCESS && result != ISC_R_NOTIMPLEMENTED) {
-			isc_refcount_decrement(&node->references);
+			isc_refcount_decrementz(&node->references);
 			destroynode(node);
 			return (result);
 		}
@@ -1299,7 +1299,7 @@ dbiterator_destroy(dns_dbiterator_t **iteratorp) {
 		dns_sdlznode_t *node;
 		node = ISC_LIST_HEAD(sdlziter->nodelist);
 		ISC_LIST_UNLINK(sdlziter->nodelist, node, link);
-		isc_refcount_decrement(&node->references);
+		isc_refcount_decrementz(&node->references);
 		destroynode(node);
 	}
 
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index 591dfca77a6..4845a8a511b 100644
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -349,7 +349,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm,
 cleanup_refs:
 	tkey->magic = 0;
 	while (refs-- > 0) {
-		isc_refcount_decrement(&tkey->refs);
+		isc_refcount_decrement0(&tkey->refs);
 	}
 	isc_refcount_destroy(&tkey->refs);
 
diff --git a/lib/dns/view.c b/lib/dns/view.c
index e0b969b8d88..d7b9f149c3b 100644
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -314,10 +314,10 @@ cleanup_dynkeys:
 	}
 
 cleanup_weakrefs:
-	isc_refcount_decrement(&view->weakrefs);
+	isc_refcount_decrementz(&view->weakrefs);
 	isc_refcount_destroy(&view->weakrefs);
 
-	isc_refcount_decrement(&view->references);
+	isc_refcount_decrementz(&view->references);
 	isc_refcount_destroy(&view->references);
 
 	if (view->fwdtable != NULL) {
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 2cd966d28b7..d6310878a89 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -1146,7 +1146,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) {
 	return (ISC_R_SUCCESS);
 
 free_refs:
-	isc_refcount_decrement(&zone->erefs);
+	isc_refcount_decrement0(&zone->erefs);
 	isc_refcount_destroy(&zone->erefs);
 	isc_refcount_destroy(&zone->irefs);
 
diff --git a/lib/dns/zt.c b/lib/dns/zt.c
index 53e491be943..acceb9cc29e 100644
--- a/lib/dns/zt.c
+++ b/lib/dns/zt.c
@@ -375,8 +375,8 @@ asyncload(dns_zone_t *zone, void *zt_) {
 		 * Caller is holding a reference to zt->loads_pending
 		 * and zt->references so these can't decrement to zero.
 		 */
-		INSIST(isc_refcount_decrement(&zt->loads_pending) > 1);
-		INSIST(isc_refcount_decrement(&zt->references) > 1);
+		isc_refcount_decrement1(&zt->references);
+		isc_refcount_decrement1(&zt->loads_pending);
 	}
 	return (ISC_R_SUCCESS);
 }
diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c
index 1bb5ec7dac0..688c873daa4 100644
--- a/lib/isc/httpd.c
+++ b/lib/isc/httpd.c
@@ -236,7 +236,7 @@ isc_httpdmgr_create(isc_nm_t *nm, isc_mem_t *mctx, isc_sockaddr_t *addr,
 
 cleanup:
 	httpdmgr->magic = 0;
-	isc_refcount_decrement(&httpdmgr->references);
+	isc_refcount_decrementz(&httpdmgr->references);
 	isc_refcount_destroy(&httpdmgr->references);
 	isc_mem_detach(&httpdmgr->mctx);
 	isc_mutex_destroy(&httpdmgr->lock);
diff --git a/lib/isc/include/isc/refcount.h b/lib/isc/include/isc/refcount.h
index 42a0c51ca23..74543d471f7 100644
--- a/lib/isc/include/isc/refcount.h
+++ b/lib/isc/include/isc/refcount.h
@@ -133,4 +133,22 @@ isc_refcount_decrement(isc_refcount_t *target) {
 	})
 #endif /* _MSC_VER */
 
+#define isc_refcount_decrementz(target)                               \
+	do {                                                          \
+		uint_fast32_t _refs = isc_refcount_decrement(target); \
+		ISC_INSIST(_refs == 1);                               \
+	} while (0)
+
+#define isc_refcount_decrement1(target)                               \
+	do {                                                          \
+		uint_fast32_t _refs = isc_refcount_decrement(target); \
+		ISC_INSIST(_refs > 1);                                \
+	} while (0)
+
+#define isc_refcount_decrement0(target)                               \
+	do {                                                          \
+		uint_fast32_t _refs = isc_refcount_decrement(target); \
+		ISC_INSIST(_refs > 0);                                \
+	} while (0)
+
 ISC_LANG_ENDDECLS
diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index ba58d02d156..2c8d594e365 100644
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -1015,7 +1015,7 @@ isc_mem_destroy(isc_mem_t **ctxp) {
 		print_active(ctx, stderr);
 	}
 #else  /* if ISC_MEM_TRACKLINES */
-	isc_refcount_decrement(&ctx->references);
+	isc_refcount_decrementz(&ctx->references);
 #endif /* if ISC_MEM_TRACKLINES */
 	isc_refcount_destroy(&ctx->references);
 	destroy(ctx);
diff --git a/lib/isc/unix/socket.c b/lib/isc/unix/socket.c
index e6952bc9aff..043b33e34df 100644
--- a/lib/isc/unix/socket.c
+++ b/lib/isc/unix/socket.c
@@ -2999,7 +2999,7 @@ internal_accept(isc__socket_t *sock) {
 		inc_stats(manager->stats, sock->statsindex[STATID_ACCEPT]);
 	} else {
 		inc_stats(manager->stats, sock->statsindex[STATID_ACCEPTFAIL]);
-		(void)isc_refcount_decrement(&NEWCONNSOCK(dev)->references);
+		isc_refcount_decrementz(&NEWCONNSOCK(dev)->references);
 		free_socket((isc__socket_t **)&dev->newsocket);
 	}
 
@@ -5081,7 +5081,7 @@ isc_socket_cancel(isc_socket_t *sock0, isc_task_t *task, unsigned int how) {
 				ISC_LIST_UNLINK(sock->accept_list, dev,
 						ev_link);
 
-				(void)isc_refcount_decrement(
+				isc_refcount_decrementz(
 					&NEWCONNSOCK(dev)->references);
 				free_socket((isc__socket_t **)&dev->newsocket);
 
diff --git a/lib/isc/win32/socket.c b/lib/isc/win32/socket.c
index af55eb843a5..917927fc730 100644
--- a/lib/isc/win32/socket.c
+++ b/lib/isc/win32/socket.c
@@ -2488,7 +2488,7 @@ SocketIoThread(LPVOID ThreadContext) {
 					closesocket(lpo->adev->newsocket->fd);
 					lpo->adev->newsocket->fd =
 						INVALID_SOCKET;
-					isc_refcount_decrement(
+					isc_refcount_decrementz(
 						&lpo->adev->newsocket
 							 ->references);
 					free_socket(&lpo->adev->newsocket,
@@ -3501,7 +3501,7 @@ isc_socket_cancel(isc_socket_t *sock, isc_task_t *task, unsigned int how) {
 			next = ISC_LIST_NEXT(dev, ev_link);
 
 			if ((task == NULL) || (task == current_task)) {
-				isc_refcount_decrement(
+				isc_refcount_decrementz(
 					&dev->newsocket->references);
 				closesocket(dev->newsocket->fd);
 				dev->newsocket->fd = INVALID_SOCKET;