From: Alessio Podda <alessio@isc.org>
Date: Thu, 16 Apr 2026 11:21:04 +0000 (+0200)
Subject: Fix strict weak ordering violation in resign_sooner()
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c7a167c7393fa0672ca6f06187c473dbe0bf5a42;p=thirdparty%2Fbind9.git

Fix strict weak ordering violation in resign_sooner()

resign_sooner_values() only checked whether rhs was SOA-typed when
resign times were equal, but did not check lhs. When both entries were
SOA-typed with equal resign times, the comparison returned true in both
directions, violating irreflexivity and corrupting heap invariants.

Add lhs_typepair parameter and require lhs to be non-SOA for the
tie-breaking logic to apply.
---

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index d7604affbab..47c274889a6 100644
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -637,13 +637,16 @@ qpdb_destroy(dns_db_t *arg) {
 /*%
  * Compare resign times with SOA priority logic.
  * Returns true if lhs should be resigned sooner than rhs.
+ * When resign times are equal, non-SOA RRsets sort before the SOA RRset.
  */
 static bool
-resign_sooner_values(int64_t lhs_resign, int64_t rhs_resign,
-		     dns_typepair_t rhs_typepair) {
+resign_sooner_values(int64_t lhs_resign, dns_typepair_t lhs_typepair,
+		     int64_t rhs_resign, dns_typepair_t rhs_typepair) {
+	bool lhs_is_soa = lhs_typepair == DNS_SIGTYPEPAIR(dns_rdatatype_soa);
+	bool rhs_is_soa = rhs_typepair == DNS_SIGTYPEPAIR(dns_rdatatype_soa);
+
 	return lhs_resign < rhs_resign ||
-	       (lhs_resign == rhs_resign &&
-		rhs_typepair == DNS_SIGTYPEPAIR(dns_rdatatype_soa));
+	       (lhs_resign == rhs_resign && lhs_is_soa < rhs_is_soa);
 }
 
 /*%
@@ -655,9 +658,9 @@ resign_sooner(void *v1, void *v2) {
 	qpz_resign_t *elem1 = v1;
 	qpz_resign_t *elem2 = v2;
 
-	return resign_sooner_values(elem1->header->resign,
-				    elem2->header->resign,
-				    elem2->header->typepair);
+	return resign_sooner_values(
+		elem1->header->resign, elem1->header->typepair,
+		elem2->header->resign, elem2->header->typepair);
 }
 
 /*%
@@ -1993,6 +1996,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 				if (loading && RESIGN(newheader) &&
 				    RESIGN(header) &&
 				    resign_sooner_values(header->resign,
+							 header->typepair,
 							 newheader->resign,
 							 newheader->typepair))
 				{
@@ -2482,13 +2486,14 @@ setsigningtime(dns_db_t *db, dns_dbnode_t *dbnode, dns_rdataset_t *rdataset,
 
 			header->resign = new_resign;
 
-			if (resign_sooner_values(new_resign, old_resign,
-						 header->typepair))
+			if (resign_sooner_values(new_resign, header->typepair,
+						 old_resign, header->typepair))
 			{
 				isc_heap_increased(qpdb->heap->heap,
 						   found_elem->heap_index);
-			} else if (resign_sooner_values(old_resign, new_resign,
-							header->typepair))
+			} else if (resign_sooner_values(
+					   old_resign, header->typepair,
+					   new_resign, header->typepair))
 			{
 				isc_heap_decreased(qpdb->heap->heap,
 						   found_elem->heap_index);
diff --git a/tests/dns/qpzone_test.c b/tests/dns/qpzone_test.c
index ebb662bc425..066f5921a53 100644
--- a/tests/dns/qpzone_test.c
+++ b/tests/dns/qpzone_test.c
@@ -387,6 +387,22 @@ ISC_RUN_TEST_IMPL(setownercase) {
 	assert_true(dns_name_caseequal(name1, name2));
 }
 
+ISC_RUN_TEST_IMPL(resign_sooner_values) {
+	dns_typepair_t soa = DNS_SIGTYPEPAIR(dns_rdatatype_soa);
+	dns_typepair_t other = DNS_SIGTYPEPAIR(dns_rdatatype_a);
+
+	UNUSED(state);
+
+	assert_true(resign_sooner_values(10, other, 20, other));
+	assert_false(resign_sooner_values(20, other, 10, other));
+
+	assert_true(resign_sooner_values(10, other, 10, soa));
+	assert_false(resign_sooner_values(10, soa, 10, other));
+
+	assert_false(resign_sooner_values(10, soa, 10, soa));
+	assert_false(resign_sooner_values(10, other, 10, other));
+}
+
 ISC_RUN_TEST_IMPL(diffop_add_sub) {
 	isc_result_t result;
 	dns_db_t *db = NULL;
@@ -503,6 +519,7 @@ ISC_RUN_TEST_IMPL(diffop_addresign) {
 ISC_TEST_LIST_START
 ISC_TEST_ENTRY(ownercase)
 ISC_TEST_ENTRY(setownercase)
+ISC_TEST_ENTRY(resign_sooner_values)
 ISC_TEST_ENTRY(diffop_add_sub)
 ISC_TEST_ENTRY(diffop_addresign)
 ISC_TEST_LIST_END