Fix handling of TAT sending failures

dns_view_zonecut() may associate the dns_rdataset_t structure passed to
it even if it returns a result different then ISC_R_SUCCESS.  Not
handling this properly may cause a reference leak.  Fix by ensuring
'nameservers' is cleaned up in all relevant failure modes.

diff --git a/bin/named/server.c b/bin/named/server.c
index a21ea705979b5d7a32652df170f6e60e435d5d9e..97b7b8dd7e3e7cbaaaad5e6125fac21e97d5c3d9 100644 (file)
--- a/bin/named/server.c
+++ b/bin/named/server.c
@@ -6646,25 +6646,32 @@ dotat(dns_keytable_t *keytable, dns_keynode_t *keynode, void *arg) {
        dns_rdataset_init(&nameservers);
        result = dns_view_findzonecut(view, origin, domain, 0, 0, ISC_TRUE,
                                      ISC_TRUE, &nameservers, NULL);
-       if (result != ISC_R_SUCCESS) {
-               goto done;
+       if (result == ISC_R_SUCCESS) {
+               result = dns_resolver_createfetch(view->resolver, tatname,
+                                                 dns_rdatatype_null, domain,
+                                                 &nameservers, NULL, NULL, 0,
+                                                 0, 0, NULL, tat->task,
+                                                 tat_done, tat,
+                                                 &tat->rdataset,
+                                                 &tat->sigrdataset,
+                                                 &tat->fetch);
        }
 
-       result = dns_resolver_createfetch(view->resolver, tatname,
-                                         dns_rdatatype_null, domain,
-                                         &nameservers, NULL, NULL, 0, 0, 0,
-                                         NULL, tat->task, tat_done, tat,
-                                         &tat->rdataset, &tat->sigrdataset,
-                                         &tat->fetch);
-
        /*
-        * dns_resolver_createfetch() creates its own copies of 'domain' and
-        * 'nameservers'; clean up the latter (the former points into a
-        * dst_key_t structure and thus must not be freed).
+        * 'domain' holds the dns_name_t pointer inside a dst_key_t structure.
+        * dns_resolver_createfetch() creates its own copy of 'domain' if it
+        * succeeds.  Thus, 'domain' is not freed here.
+        *
+        * Even if dns_view_findzonecut() returned something else than
+        * ISC_R_SUCCESS, it still could have associated 'nameservers'.
+        * dns_resolver_createfetch() creates its own copy of 'nameservers' if
+        * it succeeds.  Thus, we need to check whether 'nameservers' is
+        * associated and release it if it is.
         */
-       dns_rdataset_disassociate(&nameservers);
+       if (dns_rdataset_isassociated(&nameservers)) {
+               dns_rdataset_disassociate(&nameservers);
+       }
 
- done:
        if (result != ISC_R_SUCCESS) {
                isc_task_detach(&tat->task);
                isc_mem_putanddetach(&tat->mctx, tat, sizeof(*tat));

diff --git a/bin/tests/system/mirror/ns1/sign.sh b/bin/tests/system/mirror/ns1/sign.sh
index 4a09a8619bdde3aef25779e56fa04fcb960e711c..0382585541a2a2e3bdbc6da1b8ee450abd7a4091 100644 (file)
--- a/bin/tests/system/mirror/ns1/sign.sh
+++ b/bin/tests/system/mirror/ns1/sign.sh
@@ -27,4 +27,10 @@ cat $infile $keyname1.key $keyname2.key > $zonefile
 
 $SIGNER -P -g -o $zone $zonefile > /dev/null
 
-keyfile_to_trusted_keys $keyname1 > trusted.conf
+# Add a trust anchor for a name whose non-existence can be securely proved
+# without recursing when the root zone is mirrored.  This will exercise code
+# attempting to send TAT queries for such names (in ns3).  Key data is
+# irrelevant here, so just reuse the root zone key generated above.
+sed "s/^\./nonexistent./;" $keyname1.key > $keyname1.modified.key
+
+keyfile_to_trusted_keys $keyname1 $keyname1.modified > trusted.conf