From: Ondřej Surý <ondrej@isc.org>
Date: Wed, 13 Aug 2025 06:45:45 +0000 (+0200)
Subject: Always return DNS_R_UNCHANGED when new slabheader was not added
X-Git-Tag: v9.21.12~41^2~1
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aa3408235abf02654ccf703ad8d0031adc211bd7;p=thirdparty%2Fbind9.git

Always return DNS_R_UNCHANGED when new slabheader was not added

Change the add() function in the dns_qpcache to properly return
DNS_R_UNCHANGED if the newheader was not actually consumed, and move
the dns_slabheader_destroy() call outside of the add() function.
---

diff --git a/lib/dns/qpcache.c b/lib/dns/qpcache.c
index c5de17203a0..c61b57a50af 100644
--- a/lib/dns/qpcache.c
+++ b/lib/dns/qpcache.c
@@ -55,6 +55,7 @@
 #include <dns/rdatatype.h>
 #include <dns/stats.h>
 #include <dns/time.h>
+#include <dns/types.h>
 #include <dns/view.h>
 
 #include "db_p.h"
@@ -841,9 +842,9 @@ static void
 mark(dns_slabheader_t *header, uint_least16_t flag) {
 	uint_least16_t attributes = atomic_load_acquire(&header->attributes);
 	uint_least16_t newattributes = 0;
+	qpcache_t *qpdb = HEADERNODE(header)->qpdb;
 	dns_stats_t *stats = NULL;
 
-	qpcache_t *qpdb = HEADERNODE(header)->qpdb;
 	/*
 	 * If we are already ancient there is nothing to do.
 	 */
@@ -860,10 +861,8 @@ mark(dns_slabheader_t *header, uint_least16_t flag) {
 	 * RRtype.
 	 */
 	stats = dns_db_getrrsetstats(&qpdb->common);
-	if (stats != NULL) {
-		update_rrsetstats(stats, header->typepair, attributes, false);
-		update_rrsetstats(stats, header->typepair, newattributes, true);
-	}
+	update_rrsetstats(stats, header->typepair, attributes, false);
+	update_rrsetstats(stats, header->typepair, newattributes, true);
 }
 
 static void
@@ -2654,7 +2653,6 @@ add(qpcache_t *qpdb, qpcnode_t *qpnode,
 					 * The NXDOMAIN/NODATA(QTYPE=ANY)
 					 * is more trusted.
 					 */
-					dns_slabheader_destroy(&newheader);
 					if (addedrdataset != NULL) {
 						bindrdataset(
 							qpdb, qpnode, topheader,
@@ -2703,7 +2701,6 @@ find_header:
 		 * Deleting an already non-existent rdataset has no effect.
 		 */
 		if (!EXISTS(header) && !EXISTS(newheader)) {
-			dns_slabheader_destroy(&newheader);
 			return DNS_R_UNCHANGED;
 		}
 
@@ -2717,7 +2714,6 @@ find_header:
 		if (trust < header->trust &&
 		    (ACTIVE(header, now) || !EXISTS(header)))
 		{
-			dns_slabheader_destroy(&newheader);
 			if (addedrdataset != NULL) {
 				bindrdataset(qpdb, qpnode, header, now,
 					     nlocktype, tlocktype,
@@ -2764,13 +2760,12 @@ find_header:
 				header->closest = newheader->closest;
 				newheader->closest = NULL;
 			}
-			dns_slabheader_destroy(&newheader);
 			if (addedrdataset != NULL) {
 				bindrdataset(qpdb, qpnode, header, now,
 					     nlocktype, tlocktype,
 					     addedrdataset DNS__DB_FLARG_PASS);
 			}
-			return ISC_R_SUCCESS;
+			return DNS_R_UNCHANGED;
 		}
 
 		/*
@@ -2819,13 +2814,12 @@ find_header:
 				header->closest = newheader->closest;
 				newheader->closest = NULL;
 			}
-			dns_slabheader_destroy(&newheader);
 			if (addedrdataset != NULL) {
 				bindrdataset(qpdb, qpnode, header, now,
 					     nlocktype, tlocktype,
 					     addedrdataset DNS__DB_FLARG_PASS);
 			}
-			return ISC_R_SUCCESS;
+			return DNS_R_UNCHANGED;
 		}
 
 		qpcache_miss(qpdb, newheader, &nlocktype,
@@ -2848,7 +2842,6 @@ find_header:
 		 * The type already doesn't exist; no point trying
 		 * to delete it.
 		 */
-		dns_slabheader_destroy(&newheader);
 		return DNS_R_UNCHANGED;
 	} else {
 		/* No rdatasets of the given type exist at the node. */
@@ -3051,7 +3044,6 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		result = addnoqname(qpnode->mctx, newheader, qpdb->maxrrperset,
 				    rdataset);
 		if (result != ISC_R_SUCCESS) {
-			dns_slabheader_destroy(&newheader);
 			return result;
 		}
 	}
@@ -3059,7 +3051,6 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		result = addclosest(qpnode->mctx, newheader, qpdb->maxrrperset,
 				    rdataset);
 		if (result != ISC_R_SUCCESS) {
-			dns_slabheader_destroy(&newheader);
 			return result;
 		}
 	}
@@ -3125,8 +3116,12 @@ qpcache_addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 	result = add(qpdb, qpnode, name, newheader, options, addedrdataset, now,
 		     nlocktype, tlocktype DNS__DB_FLARG_PASS);
 
-	if (result == ISC_R_SUCCESS && delegating) {
-		qpnode->delegating = 1;
+	if (result == ISC_R_SUCCESS) {
+		if (delegating) {
+			qpnode->delegating = 1;
+		}
+	} else {
+		dns_slabheader_destroy(&newheader);
 	}
 
 	NODE_UNLOCK(nlock, &nlocktype);
@@ -3181,6 +3176,9 @@ qpcache_deleterdataset(dns_db_t *db, dns_dbnode_t *node,
 	NODE_WRLOCK(nlock, &nlocktype);
 	result = add(qpdb, qpnode, NULL, newheader, DNS_DBADD_FORCE, NULL, 0,
 		     nlocktype, isc_rwlocktype_none DNS__DB_FLARG_PASS);
+	if (result != ISC_R_SUCCESS) {
+		dns_slabheader_destroy(&newheader);
+	}
 	NODE_UNLOCK(nlock, &nlocktype);
 
 	return result;