Replace existing NTA instead of reusing it in dns_ntatable_add()

When an NTA already exists for a name, the old code retrieved
and reused the existing NTA object, then reset its timer via
settimer().  This is incorrect because isc_timer_start() and
isc_timer_stop() require the timer to be manipulated from its
owning loop (enforced by REQUIRE(timer->loop == isc_loop()) in
lib/isc/timer.c), and the caller may be running on a different
loop than the one that created the original NTA.

Instead, delete the old NTA (shutting down its timer on the
correct loop) and insert a fresh one that is owned by the
current loop.

diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index ec5b2c80e293507adccc6517711e642bfbd5c761..83e9c38e0105e1d916b7c5e43db2f5972ab08305 100644 (file)
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -300,8 +300,8 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
                 isc_stdtime_t now, uint32_t lifetime) {
        isc_result_t result = ISC_R_SUCCESS;
        dns__nta_t *nta = NULL;
+       dns__nta_t *old_nta = NULL;
        dns_qp_t *qp = NULL;
-       void *pval = NULL;
 
        REQUIRE(VALID_NTATABLE(ntatable));
 
@@ -317,17 +317,15 @@ dns_ntatable_add(dns_ntatable_t *ntatable, const dns_name_t *name, bool force,
        result = dns_qp_insert(qp, nta, 0);
        switch (result) {
        case ISC_R_EXISTS:
-               result = dns_qp_getname(qp, &nta->name, &pval, NULL);
-               if (result == ISC_R_SUCCESS) {
-                       /*
-                        * an NTA already existed: throw away the
-                        * new one and update the old one.
-                        */
-                       dns__nta_detach(&nta); /* for nta_create */
-                       nta = pval;
-                       break;
-               }
-               /* update the NTA's timer as if it were new */
+               result = dns_qp_deletename(qp, name, (void *)&old_nta, NULL);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+
+               dns__nta_shutdown(old_nta);
+               dns__nta_detach(&old_nta);
+
+               result = dns_qp_insert(qp, nta, 0);
+               RUNTIME_CHECK(result == ISC_R_SUCCESS);
+
                FALLTHROUGH;
        case ISC_R_SUCCESS:
                nta->expiry = now + lifetime;
@@ -381,9 +379,7 @@ delete_expired(void *arg) {
        RWLOCK(&ntatable->rwlock, isc_rwlocktype_write);
        dns_qpmulti_write(ntatable->table, &qp);
        result = dns_qp_getname(qp, &nta->name, &pval, NULL);
-       if (result == ISC_R_SUCCESS &&
-           ((dns__nta_t *)pval)->expiry == nta->expiry && !nta->shuttingdown)
-       {
+       if (result == ISC_R_SUCCESS && pval == nta && !nta->shuttingdown) {
                char nb[DNS_NAME_FORMATSIZE];
                dns_name_format(&nta->name, nb, sizeof(nb));
                isc_log_write(dns_lctx, DNS_LOGCATEGORY_DNSSEC,