Fix extra detach when dns_validator create_fetch() detects deadlock

When create_fetch() in the dns_validator unit detects deadlock, it
returns DNS_R_NOVALIDSIG, but it didn't attach to the validator.  The
other condition to returning result != ISC_R_SUCCESS would be error from
dns_resolver_createfetch().  The caller (in two places out of three)
would detect the error condition and always detach from the validator.

Move the dns_validator_detach() on dns_resolver_createfetch() error
condition to create_fetch() function and cleanup the extra detaches in
seek_dnskey() and get_dsset().

diff --git a/lib/dns/include/dns/validator.h b/lib/dns/include/dns/validator.h
index 19eae8bb5a85aa83ee445e75a2d1838aef8e2720..d42cdca637659c7624cc7ea5f0e344cc0ccca243 100644 (file)
--- a/lib/dns/include/dns/validator.h
+++ b/lib/dns/include/dns/validator.h
@@ -165,7 +165,11 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
 /*%<
  * Start a DNSSEC validation.
  *
- * This validates a response to the question given by
+ * On success (which is guaranteed as long as the view has valid
+ * trust anchors), `validatorp` is updated to point to the new
+ * validator. The caller is responsible for detaching it.
+ *
+ * The validator will validate a response to the question given by
  * 'name' and 'type'.
  *
  * To validate a positive response, the response data is

diff --git a/lib/dns/validator.c b/lib/dns/validator.c
index 617f03124e8ed2a0166638643476e7913892289c..3447d0482b30f5af89e3fb4d6a2fc3b3ce7672ba 100644 (file)
--- a/lib/dns/validator.c
+++ b/lib/dns/validator.c
@@ -926,6 +926,7 @@ static isc_result_t
 create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
             isc_job_cb callback, const char *caller) {
        unsigned int fopts = 0;
+       isc_result_t result;
 
        disassociate_rdatasets(val);
 
@@ -946,10 +947,16 @@ create_fetch(dns_validator_t *val, dns_name_t *name, dns_rdatatype_t type,
        validator_logcreate(val, name, type, caller, "fetch");
 
        dns_validator_ref(val);
-       return (dns_resolver_createfetch(
+
+       result = dns_resolver_createfetch(
                val->view->resolver, name, type, NULL, NULL, NULL, NULL, 0,
                fopts, 0, NULL, val->loop, callback, val, &val->frdataset,
-               &val->fsigrdataset, &val->fetch));
+               &val->fsigrdataset, &val->fetch);
+       if (result != ISC_R_SUCCESS) {
+               dns_validator_detach(&val);
+       }
+
+       return (result);
 }
 
 /*%
@@ -1192,7 +1199,6 @@ seek_dnskey(dns_validator_t *val) {
                                      dns_rdatatype_dnskey,
                                      fetch_callback_dnskey, "seek_dnskey");
                if (result != ISC_R_SUCCESS) {
-                       dns_validator_detach(&val);
                        return (result);
                }
                return (DNS_R_WAIT);
@@ -1646,7 +1652,6 @@ get_dsset(dns_validator_t *val, dns_name_t *tname, isc_result_t *resp) {
                                      fetch_callback_ds, "validate_dnskey");
                *resp = DNS_R_WAIT;
                if (result != ISC_R_SUCCESS) {
-                       dns_validator_detach(&val);
                        *resp = result;
                }
                return (ISC_R_COMPLETE);
@@ -3004,12 +3009,18 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                     dns_validator_t **validatorp) {
        isc_result_t result = ISC_R_FAILURE;
        dns_validator_t *val = NULL;
+       dns_keytable_t *kt = NULL;
 
        REQUIRE(name != NULL);
        REQUIRE(rdataset != NULL ||
                (rdataset == NULL && sigrdataset == NULL && message != NULL));
        REQUIRE(validatorp != NULL && *validatorp == NULL);
 
+       result = dns_view_getsecroots(view, &kt);
+       if (result != ISC_R_SUCCESS) {
+               return (result);
+       }
+
        val = isc_mem_get(view->mctx, sizeof(*val));
        *val = (dns_validator_t){ .tid = isc_tid(),
                                  .result = ISC_R_FAILURE,
@@ -3018,22 +3029,17 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
                                  .name = name,
                                  .type = type,
                                  .options = options,
+                                 .keytable = kt,
                                  .link = ISC_LINK_INITIALIZER,
                                  .loop = loop,
                                  .cb = cb,
                                  .arg = arg };
 
        isc_refcount_init(&val->references, 1);
-
+       dns_view_attach(view, &val->view);
        if (message != NULL) {
                dns_message_attach(message, &val->message);
        }
-       dns_view_attach(view, &val->view);
-
-       result = dns_view_getsecroots(val->view, &val->keytable);
-       if (result != ISC_R_SUCCESS) {
-               goto cleanup;
-       }
 
        val->mustbesecure = dns_resolver_getmustbesecure(view->resolver, name);
        dns_rdataset_init(&val->fdsset);
@@ -3052,15 +3058,6 @@ dns_validator_create(dns_view_t *view, dns_name_t *name, dns_rdatatype_t type,
        *validatorp = val;
 
        return (ISC_R_SUCCESS);
-
-cleanup:
-       if (val->message != NULL) {
-               dns_message_detach(&val->message);
-       }
-       isc_mem_put(view->mctx, val, sizeof(*val));
-       dns_view_detach(&view);
-
-       return (result);
 }
 
 void