[9.20] fix: dev: Acquire the database reference before possibly last node release

Acquire the database reference in the detachnode() to prevent the last
reference to be release while the NODE_LOCK being locked.  The NODE_LOCK
is locked/unlocked inside the RCU critical section, thus it is most
probably this should not pose a problem as the database uses call_rcu
memory reclamation, but this it is still safer to acquire the reference
before releasing the node.

Closes #5194

Backport of MR !10155

Merge branch 'backport-5194-fix-assertion-failure-while-reference-counting-qpdb-9.20' into 'bind-9.20'

See merge request isc-projects/bind9!10156

lib/dns/qpcache.c

@@ -315,7 +315,7 @@ struct qpcache {
 
 #ifdef DNS_DB_NODETRACE
 #define qpcache_ref(ptr)   qpcache__ref(ptr, __func__, __FILE__, __LINE__)
-#define qpcache_unref(ptr) qpcache_unref(ptr, __func__, __FILE__, __LINE__)
+#define qpcache_unref(ptr) qpcache__unref(ptr, __func__, __FILE__, __LINE__)
 #define qpcache_attach(ptr, ptrp) \
 	qpcache__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
 #define qpcache_detach(ptrp) qpcache__detach(ptrp, __func__, __FILE__, __LINE__)
@@ -787,6 +787,10 @@ qpcnode_release(qpcache_t *qpdb, qpcnode_t *node, isc_rwlocktype_t *nlocktypep,
 		 * acquired a reference in the meantime, so we increment
 		 * erefs (but NOT references!), upgrade the node lock,
 		 * decrement erefs again, and see if it's still zero.
+		 *
+		 * We can't really assume anything about the result code of
+		 * erefs_increment.  If another thread acquires reference it
+		 * will be larger than 0, if it doesn't it is going to be 0.
 		 */
 		isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
 		qpcnode_erefs_increment(qpdb, node, *nlocktypep,
@@ -2609,7 +2613,8 @@ cleanup_deadnodes(void *arg) {
 	RUNTIME_CHECK(isc_queue_splice(&deadnodes,
 				       &qpdb->buckets[locknum].deadnodes));
 	isc_queue_for_each_entry_safe(&deadnodes, qpnode, qpnext, deadlink) {
-		qpcnode_release(qpdb, qpnode, &nlocktype, &tlocktype, false);
+		qpcnode_release(qpdb, qpnode, &nlocktype, &tlocktype,
+				false DNS__DB_FILELINE);
 	}
 
 	NODE_UNLOCK(nlock, &nlocktype);
@@ -2722,16 +2727,19 @@ detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) {
 	nlock = &qpdb->buckets[node->locknum].lock;
 
 	/*
-	 * We can't destroy qpcache while holding a nodelock, so
+	 * We can't destroy qpcache while holding a nodelock, so we need to
-	 * we need to reference it before acquiring the lock
+	 * reference it before acquiring the lock and release it afterward.
-	 * and release it afterward.
+	 * Additionally, we must ensure that we don't destroy the database while
+	 * the NODE_LOCK is locked.
 	 */
 	qpcache_ref(qpdb);
 
+	rcu_read_lock();
 	NODE_RDLOCK(nlock, &nlocktype);
 	qpcnode_release(qpdb, node, &nlocktype, &tlocktype,
 			true DNS__DB_FLARG_PASS);
 	NODE_UNLOCK(nlock, &nlocktype);
+	rcu_read_unlock();
 
 	qpcache_detach(&qpdb);
 }

lib/dns/qpzone.c

@@ -186,7 +186,7 @@ struct qpznode {
 	void *data;
 };
 
-typedef struct qpcache_bucket {
+typedef struct qpzone_bucket {
 	/* Per-bucket lock. */
 	isc_rwlock_t lock;
 
@@ -246,7 +246,7 @@ struct qpzonedb {
 
 #ifdef DNS_DB_NODETRACE
 #define qpzonedb_ref(ptr)   qpzonedb__ref(ptr, __func__, __FILE__, __LINE__)
-#define qpzonedb_unref(ptr) qpzonedb_unref(ptr, __func__, __FILE__, __LINE__)
+#define qpzonedb_unref(ptr) qpzonedb__unref(ptr, __func__, __FILE__, __LINE__)
 #define qpzonedb_attach(ptr, ptrp) \
 	qpzonedb__attach(ptr, ptrp, __func__, __FILE__, __LINE__)
 #define qpzonedb_detach(ptrp) \
@@ -870,6 +870,10 @@ qpznode_release(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
 		 * acquired a reference in the meantime, so we increment
 		 * erefs (but NOT references!), upgrade the node lock,
 		 * decrement erefs again, and see if it's still zero.
+		 *
+		 * We can't really assume anything about the result code of
+		 * erefs_increment.  If another thread acquires reference it
+		 * will be larger than 0, if it doesn't it is going to be 0.
 		 */
 		isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
 		qpznode_erefs_increment(qpdb, node DNS__DB_FLARG_PASS);
@@ -3853,15 +3857,21 @@ detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) {
 	nlock = &qpdb->buckets[node->locknum].lock;
 
 	/*
-	 * qpzone_destroy() uses call_rcu() API to destroy the node locks,
+	 * qpzone_destroy() uses call_rcu() API to destroy the node locks, so it
-	 * so it is safe to call it in the middle of NODE_LOCK.
+	 * is safe to call it in the middle of NODE_LOCK, but we need to acquire
+	 * the database reference to prevent destroying the database while the
+	 * NODE_LOCK is locked.
 	 */
 
+	qpzonedb_ref(qpdb);
+
 	rcu_read_lock();
 	NODE_RDLOCK(nlock, &nlocktype);
 	qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
 	NODE_UNLOCK(nlock, &nlocktype);
 	rcu_read_unlock();
+
+	qpzonedb_unref(qpdb);
 }
 
 static unsigned int