Merge branch '4234-confidential-use-hashmap-when-parsing' into 'v9.19.20-release'

[CVE-2023-4408] Use hashmap when parsing DNS messages

See merge request isc-private/bind9!560

CHANGES

@@ -6,7 +6,8 @@
 
 6316.	[placeholder]
 
-6315.	[placeholder]
+6315.	[security]	Speed up parsing of DNS messages with many different
+			names. (CVE-2023-4408) [GL #4234]
 
 6314.	[bug]		Address race conditions in dns_tsigkey_find().
 			[GL #4182]

doc/notes/notes-current.rst

@@ -15,7 +15,13 @@ Notes for BIND 9.19.20
 Security Fixes
 ~~~~~~~~~~~~~~
 
-- None.
+- Parsing DNS messages with many different names could cause excessive
+  CPU load. This has been fixed. :cve:`2023-4408`
+
+  ISC would like to thank Shoham Danino from Reichman University, Anat
+  Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv
+  University, and Yuval Shavitt from Tel-Aviv University for bringing
+  this vulnerability to our attention. :gl:`#4234`
 
 New Features
 ~~~~~~~~~~~~

lib/dns/include/dns/message.h

@@ -859,27 +859,6 @@ dns_message_findtype(const dns_name_t *name, dns_rdatatype_t type,
  *\li	#ISC_R_NOTFOUND		-- the desired type does not exist.
  */
 
-isc_result_t
-dns_message_find(const dns_name_t *name, dns_rdataclass_t rdclass,
-		 dns_rdatatype_t type, dns_rdatatype_t covers,
-		 dns_rdataset_t **rdataset);
-/*%<
- * Search the name for the specified rdclass and type.  If it is found,
- * *rdataset is filled in with a pointer to that rdataset.
- *
- * Requires:
- *\li	if '**rdataset' is non-NULL, *rdataset needs to be NULL.
- *
- *\li	'type' be a valid type, and NOT dns_rdatatype_any.
- *
- *\li	If 'type' is dns_rdatatype_rrsig, 'covers' must be a valid type.
- *	Otherwise it should be 0.
- *
- * Returns:
- *\li	#ISC_R_SUCCESS		-- all is well.
- *\li	#ISC_R_NOTFOUND		-- the desired type does not exist.
- */
-
 void
 dns_message_addname(dns_message_t *msg, dns_name_t *name,
 		    dns_section_t section);

lib/dns/include/dns/name.h

@@ -68,6 +68,7 @@
 #include <stdio.h>
 
 #include <isc/buffer.h>
+#include <isc/hashmap.h>
 #include <isc/lang.h>
 #include <isc/magic.h>
 #include <isc/region.h> /* Required for storage size of dns_label_t. */
@@ -119,6 +120,7 @@ struct dns_name {
 	isc_buffer_t  *buffer;
 	ISC_LINK(dns_name_t) link;
 	ISC_LIST(dns_rdataset_t) list;
+	isc_hashmap_t *hashmap;
 };
 
 #define DNS_NAME_MAGIC	  ISC_MAGIC('D', 'N', 'S', 'n')

lib/dns/message.c

@@ -22,6 +22,8 @@
 #include <stdbool.h>
 
 #include <isc/buffer.h>
+#include <isc/hash.h>
+#include <isc/hashmap.h>
 #include <isc/mem.h>
 #include <isc/result.h>
 #include <isc/string.h>
@@ -779,6 +781,11 @@ ISC_REFCOUNT_TRACE_IMPL(dns_message, dns__message_destroy);
 ISC_REFCOUNT_IMPL(dns_message, dns__message_destroy);
 #endif
 
+static bool
+name_match(void *node, const void *key) {
+	return (dns_name_equal(node, key));
+}
+
 static isc_result_t
 findname(dns_name_t **foundname, const dns_name_t *target,
 	 dns_namelist_t *section) {
@@ -796,26 +803,25 @@ findname(dns_name_t **foundname, const dns_name_t *target,
 	return (ISC_R_NOTFOUND);
 }
 
-isc_result_t
+static uint32_t
-dns_message_find(const dns_name_t *name, dns_rdataclass_t rdclass,
+rds_hash(dns_rdataset_t *rds) {
-		 dns_rdatatype_t type, dns_rdatatype_t covers,
+	isc_hash32_t state;
-		 dns_rdataset_t **rdatasetp) {
-	dns_rdataset_t *rds = NULL;
 
-	REQUIRE(name != NULL);
+	isc_hash32_init(&state);
-	REQUIRE(rdatasetp == NULL || *rdatasetp == NULL);
+	isc_hash32_hash(&state, &rds->rdclass, sizeof(rds->rdclass), true);
+	isc_hash32_hash(&state, &rds->type, sizeof(rds->type), true);
+	isc_hash32_hash(&state, &rds->covers, sizeof(rds->covers), true);
 
-	ISC_LIST_FOREACH_REV (name->list, rds, link) {
+	return (isc_hash32_finalize(&state));
-		if (rds->rdclass == rdclass && rds->type == type &&
-		    rds->covers == covers)
-		{
-			SET_IF_NOT_NULL(rdatasetp, rds);
-
-			return (ISC_R_SUCCESS);
-		}
 }
 
-	return (ISC_R_NOTFOUND);
+static bool
+rds_match(void *node, const void *key0) {
+	const dns_rdataset_t *rds = node;
+	const dns_rdataset_t *key = key0;
+
+	return (rds->rdclass == key->rdclass && rds->type == key->type &&
+		rds->covers == key->covers);
 }
 
 isc_result_t
@@ -939,22 +945,38 @@ getrdata(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		}                            \
 	} while (0)
 
+static void
+cleanup_name_hashmaps(dns_namelist_t *section) {
+	dns_name_t *name = NULL;
+	ISC_LIST_FOREACH (*section, name, link) {
+		if (name->hashmap != NULL) {
+			isc_hashmap_destroy(&name->hashmap);
+		}
+	}
+}
+
 static isc_result_t
 getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	     unsigned int options) {
 	isc_region_t r;
 	unsigned int count;
 	dns_name_t *name = NULL;
-	dns_name_t *name2 = NULL;
+	dns_name_t *found_name = NULL;
 	dns_rdataset_t *rdataset = NULL;
 	dns_rdatalist_t *rdatalist = NULL;
-	isc_result_t result;
+	isc_result_t result = ISC_R_SUCCESS;
 	dns_rdatatype_t rdtype;
 	dns_rdataclass_t rdclass;
 	dns_namelist_t *section = &msg->sections[DNS_SECTION_QUESTION];
 	bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0);
 	bool seen_problem = false;
 	bool free_name = false;
+	bool free_hashmaps = false;
+	isc_hashmap_t *name_map = NULL;
+
+	if (msg->counts[DNS_SECTION_QUESTION] > 1) {
+		isc_hashmap_create(msg->mctx, 1, &name_map);
+	}
 
 	for (count = 0; count < msg->counts[DNS_SECTION_QUESTION]; count++) {
 		name = NULL;
@@ -972,13 +994,21 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			goto cleanup;
 		}
 
+		/* If there is only one QNAME, skip the duplicity checks */
+		if (name_map == NULL) {
+			result = ISC_R_SUCCESS;
+			goto skip_name_check;
+		}
+
 		/*
 		 * Run through the section, looking to see if this name
 		 * is already there.  If it is found, put back the allocated
 		 * name since we no longer need it, and set our name pointer
 		 * to point to the name we found.
 		 */
-		result = findname(&name2, name, section);
+		result = isc_hashmap_add(name_map, dns_name_hash(name),
+					 name_match, name, name,
+					 (void **)&found_name);
 
 		/*
 		 * If it is the first name in the section, accept it.
@@ -990,19 +1020,25 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		 * this should be legal or not.  In either case we no longer
 		 * need this name pointer.
 		 */
-		if (result != ISC_R_SUCCESS) {
+	skip_name_check:
+		switch (result) {
+		case ISC_R_SUCCESS:
 			if (!ISC_LIST_EMPTY(*section)) {
 				DO_ERROR(DNS_R_FORMERR);
 			}
 			ISC_LIST_APPEND(*section, name, link);
-			free_name = false;
+			break;
-		} else {
+		case ISC_R_EXISTS:
 			dns_message_puttempname(msg, &name);
-			name = name2;
+			name = found_name;
-			name2 = NULL;
+			found_name = NULL;
-			free_name = false;
+			break;
+		default:
+			UNREACHABLE();
 		}
 
+		free_name = false;
+
 		/*
 		 * Get type and class.
 		 */
@@ -1032,53 +1068,83 @@ getquestions(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			msg->tkey = 1;
 		}
 
-		/*
-		 * Can't ask the same question twice.
-		 */
-		result = dns_message_find(name, rdclass, rdtype, 0, NULL);
-		if (result == ISC_R_SUCCESS) {
-			DO_ERROR(DNS_R_FORMERR);
-		}
-
 		/*
 		 * Allocate a new rdatalist.
 		 */
 		rdatalist = newrdatalist(msg);
-		if (rdatalist == NULL) {
+		rdatalist->type = rdtype;
-			result = ISC_R_NOMEMORY;
+		rdatalist->rdclass = rdclass;
-			goto cleanup;
+		rdatalist->covers = 0;
-		}
-		rdataset = isc_mempool_get(msg->rdspool);
 
 		/*
 		 * Convert rdatalist to rdataset, and attach the latter to
 		 * the name.
 		 */
-		rdatalist->type = rdtype;
+		dns_message_gettemprdataset(msg, &rdataset);
-		rdatalist->rdclass = rdclass;
-
-		dns_rdataset_init(rdataset);
 		dns_rdatalist_tordataset(rdatalist, rdataset);
 
 		rdataset->attributes |= DNS_RDATASETATTR_QUESTION;
 
+		/*
+		 * Skip the duplicity check for first rdataset
+		 */
+		if (ISC_LIST_EMPTY(name->list)) {
+			result = ISC_R_SUCCESS;
+			goto skip_rds_check;
+		}
+
+		/*
+		 * Can't ask the same question twice.
+		 */
+		if (name->hashmap == NULL) {
+			isc_hashmap_create(msg->mctx, 1, &name->hashmap);
+			free_hashmaps = true;
+
+			INSIST(ISC_LIST_HEAD(name->list) ==
+			       ISC_LIST_TAIL(name->list));
+
+			dns_rdataset_t *old_rdataset =
+				ISC_LIST_HEAD(name->list);
+
+			result = isc_hashmap_add(
+				name->hashmap, rds_hash(old_rdataset),
+				rds_match, old_rdataset, old_rdataset, NULL);
+
+			INSIST(result == ISC_R_SUCCESS);
+		}
+		result = isc_hashmap_add(name->hashmap, rds_hash(rdataset),
+					 rds_match, rdataset, rdataset, NULL);
+		if (result == ISC_R_EXISTS) {
+			DO_ERROR(DNS_R_FORMERR);
+		}
+
+	skip_rds_check:
 		ISC_LIST_APPEND(name->list, rdataset, link);
+
 		rdataset = NULL;
 	}
 
 	if (seen_problem) {
-		return (DNS_R_RECOVERABLE);
+		result = DNS_R_RECOVERABLE;
 	}
-	return (ISC_R_SUCCESS);
 
 cleanup:
 	if (rdataset != NULL) {
 		dns_message_puttemprdataset(msg, &rdataset);
 	}
+
 	if (free_name) {
 		dns_message_puttempname(msg, &name);
 	}
 
+	if (free_hashmaps) {
+		cleanup_name_hashmaps(section);
+	}
+
+	if (name_map != NULL) {
+		isc_hashmap_destroy(&name_map);
+	}
+
 	return (result);
 }
 
@@ -1105,7 +1171,6 @@ auth_signed(dns_namelist_t *section) {
 	ISC_LIST_FOREACH (*section, name, link) {
 		int auth_dnssec = 0, auth_rrsig = 0;
 		dns_rdataset_t *rds = NULL;
-
 		ISC_LIST_FOREACH (name->list, rds, link) {
 			switch (rds->type) {
 			case dns_rdatatype_ds:
@@ -1152,19 +1217,26 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	isc_region_t r;
 	unsigned int count, rdatalen;
 	dns_name_t *name = NULL;
-	dns_name_t *name2 = NULL;
+	dns_name_t *found_name = NULL;
 	dns_rdataset_t *rdataset = NULL;
+	dns_rdataset_t *found_rdataset = NULL;
 	dns_rdatalist_t *rdatalist = NULL;
-	isc_result_t result;
+	isc_result_t result = ISC_R_SUCCESS;
 	dns_rdatatype_t rdtype, covers;
 	dns_rdataclass_t rdclass;
 	dns_rdata_t *rdata = NULL;
 	dns_ttl_t ttl;
 	dns_namelist_t *section = &msg->sections[sectionid];
-	bool free_name = false, free_rdataset = false, seen_problem = false;
+	bool free_name = false, seen_problem = false;
+	bool free_hashmaps = false;
 	bool preserve_order = ((options & DNS_MESSAGEPARSE_PRESERVEORDER) != 0);
 	bool best_effort = ((options & DNS_MESSAGEPARSE_BESTEFFORT) != 0);
 	bool isedns, issigzero, istsig;
+	isc_hashmap_t *name_map = NULL;
+
+	if (msg->counts[sectionid] > 1) {
+		isc_hashmap_create(msg->mctx, 1, &name_map);
+	}
 
 	for (count = 0; count < msg->counts[sectionid]; count++) {
 		int recstart = source->current;
@@ -1172,7 +1244,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 
 		skip_name_search = false;
 		skip_type_search = false;
-		free_rdataset = false;
 		isedns = false;
 		issigzero = false;
 		istsig = false;
@@ -1212,8 +1283,8 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 		if (msg->rdclass_set == 0 &&
 		    rdtype != dns_rdatatype_opt &&  /* class is UDP SIZE */
 		    rdtype != dns_rdatatype_tsig && /* class is ANY */
-		    rdtype != dns_rdatatype_tkey)
+		    rdtype != dns_rdatatype_tkey)   /* class is undefined */
-		{ /* class is undefined */
+		{
 			msg->rdclass = rdclass;
 			msg->rdclass_set = 1;
 		}
@@ -1412,61 +1483,128 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 				free_name = false;
 			}
 		} else {
+			if (name_map == NULL) {
+				result = ISC_R_SUCCESS;
+				goto skip_name_check;
+			}
+
 			/*
 			 * Run through the section, looking to see if this name
 			 * is already there.  If it is found, put back the
 			 * allocated name since we no longer need it, and set
 			 * our name pointer to point to the name we found.
 			 */
-			result = findname(&name2, name, section);
+			result = isc_hashmap_add(name_map, dns_name_hash(name),
+						 name_match, name, name,
+						 (void **)&found_name);
 
 			/*
 			 * If it is a new name, append to the section.
 			 */
-			if (result == ISC_R_SUCCESS) {
+		skip_name_check:
-				dns_message_puttempname(msg, &name);
+			switch (result) {
-				name = name2;
+			case ISC_R_SUCCESS:
-			} else {
 				ISC_LIST_APPEND(*section, name, link);
+				break;
+			case ISC_R_EXISTS:
+				dns_message_puttempname(msg, &name);
+				name = found_name;
+				found_name = NULL;
+				break;
+			default:
+				UNREACHABLE();
 			}
 			free_name = false;
 		}
 
+		rdatalist = newrdatalist(msg);
+		rdatalist->type = rdtype;
+		rdatalist->covers = covers;
+		rdatalist->rdclass = rdclass;
+		rdatalist->ttl = ttl;
+
+		dns_message_gettemprdataset(msg, &rdataset);
+		dns_rdatalist_tordataset(rdatalist, rdataset);
+		dns_rdataset_setownercase(rdataset, name);
+		rdatalist = NULL;
+
 		/*
 		 * Search name for the particular type and class.
 		 * Skip this stage if in update mode or this is a meta-type.
 		 */
-		if (preserve_order || msg->opcode == dns_opcode_update ||
+		if (isedns || istsig || issigzero) {
+			/* Skip adding the rdataset to the tables */
+		} else if (preserve_order || msg->opcode == dns_opcode_update ||
 			   skip_type_search)
 		{
-			result = ISC_R_NOTFOUND;
+			result = ISC_R_SUCCESS;
+
+			ISC_LIST_APPEND(name->list, rdataset, link);
 		} else {
 			/*
 			 * If this is a type that can only occur in
 			 * the question section, fail.
 			 */
 			if (dns_rdatatype_questiononly(rdtype)) {
+				dns_message_puttemprdataset(msg, &rdataset);
 				DO_ERROR(DNS_R_FORMERR);
 			}
 
-			rdataset = NULL;
+			if (ISC_LIST_EMPTY(name->list)) {
-			result = dns_message_find(name, rdclass, rdtype, covers,
+				result = ISC_R_SUCCESS;
-						  &rdataset);
+				goto skip_rds_check;
 			}
 
+			if (name->hashmap == NULL) {
+				isc_hashmap_create(msg->mctx, 1,
+						   &name->hashmap);
+				free_hashmaps = true;
+
+				INSIST(ISC_LIST_HEAD(name->list) ==
+				       ISC_LIST_TAIL(name->list));
+
+				dns_rdataset_t *old_rdataset =
+					ISC_LIST_HEAD(name->list);
+
+				result = isc_hashmap_add(
+					name->hashmap, rds_hash(old_rdataset),
+					rds_match, old_rdataset, old_rdataset,
+					NULL);
+
+				INSIST(result == ISC_R_SUCCESS);
+			}
+
+			result = isc_hashmap_add(
+				name->hashmap, rds_hash(rdataset), rds_match,
+				rdataset, rdataset, (void **)&found_rdataset);
+
 			/*
 			 * If we found an rdataset that matches, we need to
-		 * append this rdata to that set.  If we did not, we need
+			 * append this rdata to that set.  If we did not, we
-		 * to create a new rdatalist, store the important bits there,
+			 * need to create a new rdatalist, store the important
-		 * convert it to an rdataset, and link the latter to the name.
+			 * bits there, convert it to an rdataset, and link the
-		 * Yuck.  When appending, make certain that the type isn't
+			 * latter to the name. Yuck.  When appending, make
-		 * a singleton type, such as SOA or CNAME.
+			 * certain that the type isn't a singleton type, such as
+			 * SOA or CNAME.
 			 *
-		 * Note that this check will be bypassed when preserving order,
+			 * Note that this check will be bypassed when preserving
-		 * the opcode is an update, or the type search is skipped.
+			 * order, the opcode is an update, or the type search is
+			 * skipped.
 			 */
-		if (result == ISC_R_SUCCESS) {
+		skip_rds_check:
-			if (dns_rdatatype_issingleton(rdtype)) {
+			switch (result) {
+			case ISC_R_EXISTS:
+				/* Free the rdataset we used as the key */
+				dns__message_putassociatedrdataset(msg,
+								   &rdataset);
+				rdataset = found_rdataset;
+
+				result = ISC_R_SUCCESS;
+
+				if (!dns_rdatatype_issingleton(rdtype)) {
+					break;
+				}
+
 				dns_rdata_t *first;
 				dns_rdatalist_fromrdataset(rdataset,
 							   &rdatalist);
@@ -1475,31 +1613,12 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 				if (dns_rdata_compare(rdata, first) != 0) {
 					DO_ERROR(DNS_R_FORMERR);
 				}
-			}
+				break;
-		}
+			case ISC_R_SUCCESS:
-
-		if (result == ISC_R_NOTFOUND) {
-			rdataset = isc_mempool_get(msg->rdspool);
-			free_rdataset = true;
-
-			rdatalist = newrdatalist(msg);
-			if (rdatalist == NULL) {
-				result = ISC_R_NOMEMORY;
-				goto cleanup;
-			}
-
-			rdatalist->type = rdtype;
-			rdatalist->covers = covers;
-			rdatalist->rdclass = rdclass;
-			rdatalist->ttl = ttl;
-
-			dns_rdataset_init(rdataset);
-			dns_rdatalist_tordataset(rdatalist, rdataset);
-			dns_rdataset_setownercase(rdataset, name);
-
-			if (!isedns && !istsig && !issigzero) {
 				ISC_LIST_APPEND(name->list, rdataset, link);
-				free_rdataset = false;
+				break;
+			default:
+				UNREACHABLE();
 			}
 		}
 
@@ -1535,7 +1654,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 
 			msg->opt = rdataset;
 			rdataset = NULL;
-			free_rdataset = false;
 			ercode = (dns_rcode_t)((msg->opt->ttl &
 						DNS_MESSAGE_EDNSRCODE_MASK) >>
 					       20);
@@ -1547,7 +1665,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			msg->sig0name = name;
 			msg->sigstart = recstart;
 			rdataset = NULL;
-			free_rdataset = false;
 			free_name = false;
 		} else if (istsig) {
 			msg->tsig = rdataset;
@@ -1558,7 +1675,6 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			 */
 			msg->tsigname->attributes.nocompress = true;
 			rdataset = NULL;
-			free_rdataset = false;
 			free_name = false;
 		}
 
@@ -1566,14 +1682,11 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 			if (free_name) {
 				dns_message_puttempname(msg, &name);
 			}
-			if (free_rdataset) {
+			free_name = false;
-				dns__message_putassociatedrdataset(msg,
-								   &rdataset);
-			}
-			free_name = free_rdataset = false;
 		}
 		INSIST(!free_name);
-		INSIST(!free_rdataset);
+
+		rdataset = NULL;
 	}
 
 	/*
@@ -1591,16 +1704,20 @@ getsection(isc_buffer_t *source, dns_message_t *msg, dns_decompress_t dctx,
 	}
 
 	if (seen_problem) {
-		return (DNS_R_RECOVERABLE);
+		result = DNS_R_RECOVERABLE;
 	}
-	return (ISC_R_SUCCESS);
 
 cleanup:
 	if (free_name) {
 		dns_message_puttempname(msg, &name);
 	}
-	if (free_rdataset) {
+
-		dns__message_putassociatedrdataset(msg, &rdataset);
+	if (free_hashmaps) {
+		cleanup_name_hashmaps(section);
+	}
+
+	if (name_map != NULL) {
+		isc_hashmap_destroy(&name_map);
 	}
 
 	return (result);
@@ -1661,6 +1778,7 @@ dns_message_parse(dns_message_t *msg, isc_buffer_t *source,
 	dctx = DNS_DECOMPRESS_ALWAYS;
 
 	ret = getquestions(source, msg, dctx, options);
+
 	if (ret == ISC_R_UNEXPECTEDEND && ignore_tc) {
 		goto truncated;
 	}
@@ -2349,15 +2467,13 @@ dns_message_renderreset(dns_message_t *msg) {
 		dns_message_puttempname(msg, &msg->tsigname);
 	}
 	if (msg->tsig != NULL) {
-		dns_rdataset_disassociate(msg->tsig);
+		dns__message_putassociatedrdataset(msg, &msg->tsig);
-		dns_message_puttemprdataset(msg, &msg->tsig);
 	}
 	if (msg->sig0name != NULL) {
 		dns_message_puttempname(msg, &msg->sig0name);
 	}
 	if (msg->sig0 != NULL) {
-		dns_rdataset_disassociate(msg->sig0);
+		dns__message_putassociatedrdataset(msg, &msg->sig0);
-		dns_message_puttemprdataset(msg, &msg->sig0);
 	}
 }
 
@@ -2406,7 +2522,7 @@ dns_message_findname(dns_message_t *msg, dns_section_t section,
 		     const dns_name_t *target, dns_rdatatype_t type,
 		     dns_rdatatype_t covers, dns_name_t **name,
 		     dns_rdataset_t **rdataset) {
-	dns_name_t *foundname;
+	dns_name_t *foundname = NULL;
 	isc_result_t result;
 
 	/*
@@ -2524,6 +2640,10 @@ dns_message_puttempname(dns_message_t *msg, dns_name_t **itemp) {
 	REQUIRE(!ISC_LINK_LINKED(item, link));
 	REQUIRE(ISC_LIST_HEAD(item->list) == NULL);
 
+	if (item->hashmap != NULL) {
+		isc_hashmap_destroy(&item->hashmap);
+	}
+
 	/*
 	 * we need to check this in case dns_name_dup() was used.
 	 */
@@ -2551,8 +2671,7 @@ dns_message_puttemprdata(dns_message_t *msg, dns_rdata_t **item) {
 static void
 dns__message_putassociatedrdataset(dns_message_t *msg, dns_rdataset_t **item) {
 	dns_rdataset_disassociate(*item);
-	isc_mempool_put(msg->rdspool, *item);
+	dns_message_puttemprdataset(msg, item);
-	*item = NULL;
 }
 
 void
@@ -2730,8 +2849,7 @@ dns_message_setopt(dns_message_t *msg, dns_rdataset_t *opt) {
 	return (ISC_R_SUCCESS);
 
 cleanup:
-	dns_rdataset_disassociate(opt);
+	dns__message_putassociatedrdataset(msg, &opt);
-	dns_message_puttemprdataset(msg, &opt);
 	return (result);
 }