[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 605183c34fffd8f4cc560d4a8e5fba1da6184259..1b9b5491dd8c8b03bf27830b598875f92e6392bf 100644 (file)
--- 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 d8d5535c245163690681899938d5008b7da780e4..6df9b98386353f44ae6e2a01af191df1a2a7adab 100644 (file)
--- 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 37cf31f5e255b063e899dc4e848390e47ef4a2cb..304cba758ad364431a13b6391571b13174286d40 100644 (file)
--- 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 24853a54868a07a150cd68540c10bf97db2c0f7e..2cdbfd3b615a3a67ca435e5814634b8404828ebf 100644 (file)
--- 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));