From: Evan Hunt <each@isc.org>
Date: Fri, 26 Apr 2013 01:02:52 +0000 (-0700)
Subject: [master] address two more possible acache asserts
X-Git-Tag: v9.10.0a1~403
X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=ff5ac6d4213e3e2f3f6a93db8c5e65cc170a7e2b;p=thirdparty%2Fbind9.git

[master] address two more possible acache asserts

3555.	[bug]		Address theoretical race conditions in acache.c
			(change #3553 was incomplete). [RT #33252]
---

diff --git a/CHANGES b/CHANGES
index 605183c34ff..1b9b5491dd8 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,6 @@
+3555.	[bug]		Address theoretical race conditions in acache.c
+			(change #3553 was incomplete). [RT #33252]
+
 3554.	[bug]		RRL failed to correctly rate-limit upward
 			referrals and failed to count dropped error
 			responses in the statistics. [RT #33225]
diff --git a/lib/dns/acache.c b/lib/dns/acache.c
index d8d5535c245..6df9b983863 100644
--- a/lib/dns/acache.c
+++ b/lib/dns/acache.c
@@ -868,7 +868,11 @@ acache_incremental_cleaning_action(isc_task_t *task, isc_event_t *event) {
 				if (entry != NULL) {
 					/*
 					 * If we are still in the overmem
-					 * state, keep cleaning.
+					 * state, keep cleaning.  In case we
+					 * exit from the loop immediately after
+					 * this, reset next to the head entry
+					 * as we'll expect it will be never
+					 * NULL.
 					 */
 					isc_log_write(dns_lctx,
 						      DNS_LOGCATEGORY_DATABASE,
@@ -877,6 +881,7 @@ acache_incremental_cleaning_action(isc_task_t *task, isc_event_t *event) {
 						      "acache cleaner: "
 						      "still overmem, "
 						      "reset and try again");
+					next = entry;
 					continue;
 				}
 			}
@@ -895,7 +900,7 @@ acache_incremental_cleaning_action(isc_task_t *task, isc_event_t *event) {
 	 * be the starting point in the next clean-up, and reschedule another
 	 * batch.  If it fails, just try to continue anyway.
 	 */
-	INSIST(next != NULL && next != cleaner->current_entry);
+	INSIST(next != NULL);
 	dns_acache_detachentry(&cleaner->current_entry);
 	dns_acache_attachentry(next, &cleaner->current_entry);
 
@@ -1656,12 +1661,17 @@ dns_acache_setentry(dns_acache_t *acache, dns_acacheentry_t *entry,
 	return (result);
 }
 
-void
+isc_boolean_t
 dns_acache_cancelentry(dns_acacheentry_t *entry) {
-	dns_acache_t *acache = entry->acache;
+	dns_acache_t *acache;
+	isc_boolean_t callback_active;
 
 	REQUIRE(DNS_ACACHEENTRY_VALID(entry));
-	INSIST(DNS_ACACHE_VALID(acache));
+
+	acache = entry->acache;
+	callback_active = ISC_TF(entry->cbarg != NULL);
+
+	INSIST(DNS_ACACHE_VALID(entry->acache));
 
 	LOCK(&acache->lock);
 	ACACHE_LOCK(&acache->entrylocks[entry->locknum], isc_rwlocktype_write);
@@ -1681,6 +1691,8 @@ dns_acache_cancelentry(dns_acacheentry_t *entry) {
 	ACACHE_UNLOCK(&acache->entrylocks[entry->locknum],
 		      isc_rwlocktype_write);
 	UNLOCK(&acache->lock);
+
+	return (callback_active);
 }
 
 void
diff --git a/lib/dns/include/dns/acache.h b/lib/dns/include/dns/acache.h
index 37cf31f5e25..304cba758ad 100644
--- a/lib/dns/include/dns/acache.h
+++ b/lib/dns/include/dns/acache.h
@@ -387,7 +387,7 @@ dns_acache_setentry(dns_acache_t *acache, dns_acacheentry_t *entry,
  *	ISC_R_NOTFOUND
  */
 
-void
+isc_boolean_t
 dns_acache_cancelentry(dns_acacheentry_t *entry);
 /*
  * Cancel the use of the cache entry 'entry'.  This function is supposed to
@@ -398,6 +398,9 @@ dns_acache_cancelentry(dns_acacheentry_t *entry);
  *
  * Requires:
  *	'entry' is a valid additional cache entry.
+ *
+ * Returns:
+ * 	ISC_TRUE if the entry was active when canceled
  */
 
 void
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index 24853a54868..2cdbfd3b615 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -9189,9 +9189,10 @@ acache_cancelentry(isc_mem_t *mctx, dns_acacheentry_t *entry,
 
 	cbarg = *cbargp;
 
-	dns_acache_cancelentry(entry);
-	dns_db_detachnode(cbarg->db, &cbarg->node);
-	dns_db_detach(&cbarg->db);
+	if (dns_acache_cancelentry(entry)) {
+		dns_db_detachnode(cbarg->db, &cbarg->node);
+		dns_db_detach(&cbarg->db);
+	}
 
 	isc_mem_put(mctx, cbarg, sizeof(acache_cbarg_t));