]> git.ipfire.org Git - thirdparty/bind9.git/commitdiff
Use RCU for view->adb access
authorMark Andrews <marka@isc.org>
Thu, 8 Jun 2023 06:58:45 +0000 (16:58 +1000)
committerMark Andrews <marka@isc.org>
Wed, 14 Jun 2023 09:21:28 +0000 (19:21 +1000)
view->adb may be referenced while the view is shutting down as the
zone uses a weak reference to the view and examines view->adb but
dns_view_detach call dns_adb_detach to clear view->adb.

bin/named/server.c
bin/named/statschannel.c
lib/dns/include/dns/view.h
lib/dns/resolver.c
lib/dns/view.c
lib/dns/zone.c
lib/ns/client.c

index a4dc7215fb6f0bf57cc1fc7b1dc97ed47fde1479..8a6b0657088048db264cf0a3b8451461ddd7d010 100644 (file)
@@ -4096,6 +4096,7 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
        unsigned int resolver_param;
        dns_ntatable_t *ntatable = NULL;
        const char *qminmode = NULL;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
 
@@ -4766,13 +4767,22 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
                {
                        max_adb_size = MAX_ADB_SIZE_FOR_CACHESHARE;
                        if (!nsc->adbsizeadjusted) {
-                               dns_adb_setadbsize(nsc->primaryview->adb,
-                                                  MAX_ADB_SIZE_FOR_CACHESHARE);
-                               nsc->adbsizeadjusted = true;
+                               dns_view_getadb(nsc->primaryview, &adb);
+                               if (adb != NULL) {
+                                       dns_adb_setadbsize(
+                                               adb,
+                                               MAX_ADB_SIZE_FOR_CACHESHARE);
+                                       nsc->adbsizeadjusted = true;
+                                       dns_adb_detach(&adb);
+                               }
                        }
                }
        }
-       dns_adb_setadbsize(view->adb, max_adb_size);
+       dns_view_getadb(view, &adb);
+       if (adb != NULL) {
+               dns_adb_setadbsize(adb, max_adb_size);
+               dns_adb_detach(&adb);
+       }
 
        /*
         * Set up ADB quotas
@@ -4819,7 +4829,11 @@ configure_view(dns_view_t *view, dns_viewlist_t *viewlist, cfg_obj_t *config,
                obj2 = cfg_tuple_get(obj, "discount");
                discount = (double)cfg_obj_asfixedpoint(obj2) / 100.0;
 
-               dns_adb_setquota(view->adb, fps, freq, low, high, discount);
+               dns_view_getadb(view, &adb);
+               if (adb != NULL) {
+                       dns_adb_setquota(adb, fps, freq, low, high, discount);
+                       dns_adb_detach(&adb);
+               }
        }
 
        /*
@@ -11403,7 +11417,12 @@ resume:
 
        if (dctx->cache != NULL) {
                if (dctx->dumpadb) {
-                       dns_adb_dump(dctx->view->view->adb, dctx->fp);
+                       dns_adb_t *adb = NULL;
+                       dns_view_getadb(dctx->view->view, &adb);
+                       if (adb != NULL) {
+                               dns_adb_dump(adb, dctx->fp);
+                               dns_adb_detach(&adb);
+                       }
                }
                if (dctx->dumpbad) {
                        dns_resolver_printbadcache(dctx->view->view->resolver,
@@ -16263,6 +16282,7 @@ named_server_fetchlimit(named_server_t *server, isc_lex_t *lex,
        dns_view_t *view = NULL;
        char *ptr = NULL, *viewname = NULL;
        bool first = true;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(text != NULL);
 
@@ -16290,7 +16310,8 @@ named_server_fetchlimit(named_server_t *server, isc_lex_t *lex,
                        continue;
                }
 
-               if (view->adb == NULL) {
+               dns_view_getadb(view, &adb);
+               if (adb == NULL) {
                        continue;
                }
 
@@ -16300,16 +16321,16 @@ named_server_fetchlimit(named_server_t *server, isc_lex_t *lex,
                CHECK(putstr(text, "Rate limited servers, view "));
                CHECK(putstr(text, view->name));
 
-               dns_adb_getquota(view->adb, &val, NULL, NULL, NULL, NULL);
+               dns_adb_getquota(adb, &val, NULL, NULL, NULL, NULL);
                s = snprintf(tbuf, sizeof(tbuf),
                             " (fetches-per-server %u):", val);
                if (s < 0 || (unsigned int)s > sizeof(tbuf)) {
-                       return (ISC_R_NOSPACE);
+                       CHECK(ISC_R_NOSPACE);
                }
                first = false;
                CHECK(putstr(text, tbuf));
                used = isc_buffer_usedlength(*text);
-               CHECK(dns_adb_dumpquota(view->adb, text));
+               CHECK(dns_adb_dumpquota(adb, text));
                if (used == isc_buffer_usedlength(*text)) {
                        CHECK(putstr(text, "\n  None."));
                }
@@ -16320,7 +16341,7 @@ named_server_fetchlimit(named_server_t *server, isc_lex_t *lex,
                s = snprintf(tbuf, sizeof(tbuf),
                             " (fetches-per-zone %u):", val);
                if (s < 0 || (unsigned int)s > sizeof(tbuf)) {
-                       return (ISC_R_NOSPACE);
+                       CHECK(ISC_R_NOSPACE);
                }
                CHECK(putstr(text, tbuf));
                used = isc_buffer_usedlength(*text);
@@ -16328,9 +16349,12 @@ named_server_fetchlimit(named_server_t *server, isc_lex_t *lex,
                if (used == isc_buffer_usedlength(*text)) {
                        CHECK(putstr(text, "\n  None."));
                }
+               dns_adb_detach(&adb);
        }
-
 cleanup:
+       if (adb != NULL) {
+               dns_adb_detach(&adb);
+       }
        if (isc_buffer_usedlength(*text) > 0) {
                (void)putnull(text);
        }
index 0b97cdaa7b355470b21ba40ff322821e6b2f2e1d..349bec0eef1038c9239e07b74978b5a59feb8322 100644 (file)
@@ -1772,6 +1772,7 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
        {
                isc_stats_t *istats = NULL;
                dns_stats_t *dstats = NULL;
+               dns_adb_t *adb = NULL;
 
                TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "view"));
                TRY0(xmlTextWriterWriteAttribute(writer, ISC_XMLCHAR "name",
@@ -1838,10 +1839,16 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
                TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "counters"));
                TRY0(xmlTextWriterWriteAttribute(writer, ISC_XMLCHAR "type",
                                                 ISC_XMLCHAR "adbstat"));
-               CHECK(dump_stats(
-                       dns_adb_getstats(view->adb), isc_statsformat_xml,
-                       writer, NULL, adbstats_xmldesc, dns_adbstats_max,
-                       adbstats_index, adbstat_values, ISC_STATSDUMP_VERBOSE));
+               dns_view_getadb(view, &adb);
+               if (adb != NULL) {
+                       result = dump_stats(dns_adb_getstats(adb),
+                                           isc_statsformat_xml, writer, NULL,
+                                           adbstats_xmldesc, dns_adbstats_max,
+                                           adbstats_index, adbstat_values,
+                                           ISC_STATSDUMP_VERBOSE);
+                       dns_adb_detach(&adb);
+                       CHECK(result);
+               }
                TRY0(xmlTextWriterEndElement(writer)); /* </adbstats> */
 
                /* <cachestats> */
@@ -2486,6 +2493,7 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg,
                view = ISC_LIST_HEAD(server->viewlist);
                while (view != NULL) {
                        json_object *za, *v = json_object_new_object();
+                       dns_adb_t *adb = NULL;
 
                        CHECKMEM(v);
                        json_object_object_add(viewlist, view->name, v);
@@ -2591,7 +2599,11 @@ generatejson(named_server_t *server, size_t *msglen, const char **msg,
                                json_object_object_add(res, "cachestats",
                                                       counters);
 
-                               istats = dns_adb_getstats(view->adb);
+                               dns_view_getadb(view, &adb);
+                               if (adb != NULL) {
+                                       istats = dns_adb_getstats(adb);
+                                       dns_adb_detach(&adb);
+                               }
                                if (istats != NULL) {
                                        counters = json_object_new_object();
                                        CHECKMEM(counters);
@@ -3484,8 +3496,14 @@ named_stats_dump(named_server_t *server, FILE *fp) {
        for (view = ISC_LIST_HEAD(server->viewlist); view != NULL;
             view = ISC_LIST_NEXT(view, link))
        {
-               isc_stats_t *adbstats = dns_adb_getstats(view->adb);
+               dns_adb_t *adb = NULL;
+               isc_stats_t *adbstats = NULL;
 
+               dns_view_getadb(view, &adb);
+               if (adb != NULL) {
+                       adbstats = dns_adb_getstats(adb);
+                       dns_adb_detach(&adb);
+               }
                if (adbstats == NULL) {
                        continue;
                }
index b28ececce6ea323feee31bb2afaa41e3c0c2d8bb..89dd7da6e6214088c7adac42b36560d6908e4cde 100644 (file)
@@ -1274,11 +1274,11 @@ dns_view_addtrustedkey(dns_view_t *view, dns_rdatatype_t rdtype,
  * Add a DNSSEC trusted key to a view of class 'IN'.  'rdtype' is the type
  * of the RR data for the key, either DNSKEY or DS.  'keyname' is the DNS
  * name of the key, and 'databuf' stores the RR data.
-
+ *
  * Requires:
  *
  *\li  'view' is a valid view.
-
+ *
  *\li  'view' is class 'IN'.
  *
  *\li  'keyname' is a valid name.
@@ -1307,4 +1307,15 @@ dns_view_apply(dns_view_t *view, bool stop, isc_result_t *sub,
  * \li ISC_R_SHUTTINGDOWN if the view is in the process of shutting down.
  */
 
+void
+dns_view_getadb(dns_view_t *view, dns_adb_t **adbp);
+/*%<
+ * Get the view's adb if it exist.
+ *
+ * Requires:
+ *
+ *\li  'view' is a valid view.
+ *\li  'adbp' is non-NULL and '*adbp' is NULL.
+ */
+
 ISC_LANG_ENDDECLS
index e4f98c769fa9d819f6c67e45f85facd2c93f49c4..18a440f70d3a661dac9a2dfd12cad3324e4fdc4c 100644 (file)
@@ -4694,7 +4694,7 @@ fctx_create(dns_resolver_t *res, const dns_name_t *name, dns_rdatatype_t type,
         * Attach to the view's cache and adb.
         */
        dns_db_attach(res->view->cachedb, &fctx->cache);
-       dns_adb_attach(res->view->adb, &fctx->adb);
+       dns_view_getadb(res->view, &fctx->adb);
 
        ISC_LIST_INIT(fctx->resps);
        ISC_LINK_INIT(fctx, link);
index 35c788e82bd6d840a37289f758348656c0a45a5f..55c5544964d4173da2de858a0340bd34cdb0cb01 100644 (file)
@@ -465,9 +465,14 @@ dns_view_detach(dns_view_t **viewp) {
                if (view->resolver != NULL) {
                        dns_resolver_shutdown(view->resolver);
                }
-               if (view->adb != NULL) {
-                       dns_adb_shutdown(view->adb);
+
+               rcu_read_lock();
+               adb = rcu_dereference(view->adb);
+               if (adb != NULL) {
+                       dns_adb_shutdown(adb);
                }
+               rcu_read_unlock();
+
                if (view->requestmgr != NULL) {
                        dns_requestmgr_shutdown(view->requestmgr);
                }
@@ -475,6 +480,11 @@ dns_view_detach(dns_view_t **viewp) {
                /* Swap the pointers under the lock */
                LOCK(&view->lock);
 
+               if (view->resolver != NULL) {
+                       resolver = view->resolver;
+                       view->resolver = NULL;
+               }
+
                rcu_read_lock();
                zonetable = rcu_xchg_pointer(&view->zonetable, NULL);
                if (zonetable != NULL) {
@@ -482,18 +492,9 @@ dns_view_detach(dns_view_t **viewp) {
                                dns_zt_flush(zonetable);
                        }
                }
+               adb = rcu_xchg_pointer(&view->adb, NULL);
                rcu_read_unlock();
 
-               if (view->resolver != NULL) {
-                       resolver = view->resolver;
-                       view->resolver = NULL;
-               }
-
-               if (view->adb != NULL) {
-                       adb = view->adb;
-                       view->adb = NULL;
-               }
-
                if (view->requestmgr != NULL) {
                        requestmgr = view->requestmgr;
                        view->requestmgr = NULL;
@@ -525,17 +526,18 @@ dns_view_detach(dns_view_t **viewp) {
                if (resolver != NULL) {
                        dns_resolver_detach(&resolver);
                }
-               if (adb != NULL) {
-                       dns_adb_detach(&adb);
+               if (adb != NULL || zonetable != NULL) {
+                       synchronize_rcu();
+                       if (adb != NULL) {
+                               dns_adb_detach(&adb);
+                       }
+                       if (zonetable != NULL) {
+                               dns_zt_detach(&zonetable);
+                       }
                }
                if (requestmgr != NULL) {
                        dns_requestmgr_detach(&requestmgr);
                }
-
-               if (zonetable != NULL) {
-                       synchronize_rcu();
-                       dns_zt_detach(&zonetable);
-               }
                if (mkzone != NULL) {
                        dns_zone_detach(&mkzone);
                }
@@ -1474,6 +1476,7 @@ dns_view_checksig(dns_view_t *view, isc_buffer_t *source, dns_message_t *msg) {
 isc_result_t
 dns_view_flushcache(dns_view_t *view, bool fixuponly) {
        isc_result_t result;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
 
@@ -1495,9 +1498,12 @@ dns_view_flushcache(dns_view_t *view, bool fixuponly) {
                dns_badcache_flush(view->failcache);
        }
 
-       if (view->adb) {
-               dns_adb_flush(view->adb);
+       rcu_read_lock();
+       adb = rcu_dereference(view->adb);
+       if (adb != NULL) {
+               dns_adb_flush(adb);
        }
+       rcu_read_unlock();
 
        return (ISC_R_SUCCESS);
 }
@@ -1510,13 +1516,17 @@ dns_view_flushname(dns_view_t *view, const dns_name_t *name) {
 isc_result_t
 dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) {
        isc_result_t result = ISC_R_SUCCESS;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_VIEW_VALID(view));
 
        if (tree) {
-               if (view->adb != NULL) {
-                       dns_adb_flushnames(view->adb, name);
+               rcu_read_lock();
+               adb = rcu_dereference(view->adb);
+               if (adb != NULL) {
+                       dns_adb_flushnames(adb, name);
                }
+               rcu_read_unlock();
                if (view->resolver != NULL) {
                        dns_resolver_flushbadnames(view->resolver, name);
                }
@@ -1524,9 +1534,12 @@ dns_view_flushnode(dns_view_t *view, const dns_name_t *name, bool tree) {
                        dns_badcache_flushtree(view->failcache, name);
                }
        } else {
-               if (view->adb != NULL) {
-                       dns_adb_flushname(view->adb, name);
+               rcu_read_lock();
+               adb = rcu_dereference(view->adb);
+               if (adb != NULL) {
+                       dns_adb_flushname(adb, name);
                }
+               rcu_read_unlock();
                if (view->resolver != NULL) {
                        dns_resolver_flushbadcache(view->resolver, name);
                }
@@ -2487,3 +2500,18 @@ dns_view_apply(dns_view_t *view, bool stop, isc_result_t *sub,
        rcu_read_unlock();
        return (result);
 }
+
+void
+dns_view_getadb(dns_view_t *view, dns_adb_t **adbp) {
+       dns_adb_t *adb = NULL;
+
+       REQUIRE(DNS_VIEW_VALID(view));
+       REQUIRE(adbp != NULL && *adbp == NULL);
+
+       rcu_read_lock();
+       adb = rcu_dereference(view->adb);
+       if (adb != NULL) {
+               dns_adb_attach(adb, adbp);
+       }
+       rcu_read_unlock();
+}
index 6be05191edf4cb31007e390b461d5deaae7a5e6c..983b659b8b87df66f5817ec0494775ab42e2162f 100644 (file)
@@ -10910,7 +10910,7 @@ static void
 zone_maintenance(dns_zone_t *zone) {
        isc_time_t now;
        isc_result_t result;
-       bool load_pending, exiting, dumping, viewok, notify;
+       bool load_pending, exiting, dumping, viewok = false, notify;
        bool refreshkeys, sign, resign, rekey, chain, warn_expire;
 
        REQUIRE(DNS_ZONE_VALID(zone));
@@ -10924,7 +10924,14 @@ zone_maintenance(dns_zone_t *zone) {
        LOCK_ZONE(zone);
        load_pending = DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING);
        exiting = DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING);
-       viewok = (zone->view != NULL && zone->view->adb != NULL);
+       if (!load_pending && !exiting && zone->view != NULL) {
+               dns_adb_t *adb = NULL;
+               dns_view_getadb(zone->view, &adb);
+               if (adb != NULL) {
+                       dns_adb_detach(&adb);
+                       viewok = true;
+               }
+       }
        UNLOCK_ZONE(zone);
 
        if (load_pending || exiting || !viewok) {
@@ -12165,20 +12172,22 @@ static void
 notify_find_address(dns_notify_t *notify) {
        isc_result_t result;
        unsigned int options;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_NOTIFY_VALID(notify));
        options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6 |
                  DNS_ADBFIND_RETURNLAME;
 
-       if (notify->zone->view->adb == NULL) {
+       dns_view_getadb(notify->zone->view, &adb);
+       if (adb == NULL) {
                goto destroy;
        }
 
-       result = dns_adb_createfind(notify->zone->view->adb, notify->zone->loop,
-                                   process_notify_adb_event, notify,
-                                   &notify->ns, dns_rootname, 0, options, 0,
-                                   NULL, notify->zone->view->dstport, 0, NULL,
-                                   &notify->find);
+       result = dns_adb_createfind(
+               adb, notify->zone->loop, process_notify_adb_event, notify,
+               &notify->ns, dns_rootname, 0, options, 0, NULL,
+               notify->zone->view->dstport, 0, NULL, &notify->find);
+       dns_adb_detach(&adb);
 
        /* Something failed? */
        if (result != ISC_R_SUCCESS) {
@@ -20384,20 +20393,22 @@ static void
 checkds_find_address(dns_checkds_t *checkds) {
        isc_result_t result;
        unsigned int options;
+       dns_adb_t *adb = NULL;
 
        REQUIRE(DNS_CHECKDS_VALID(checkds));
        options = DNS_ADBFIND_WANTEVENT | DNS_ADBFIND_INET | DNS_ADBFIND_INET6 |
                  DNS_ADBFIND_RETURNLAME;
 
-       if (checkds->zone->view->adb == NULL) {
+       dns_view_getadb(checkds->zone->view, &adb);
+       if (adb == NULL) {
                goto destroy;
        }
 
        result = dns_adb_createfind(
-               checkds->zone->view->adb, checkds->zone->loop,
-               process_checkds_adb_event, checkds, &checkds->ns, dns_rootname,
-               0, options, 0, NULL, checkds->zone->view->dstport, 0, NULL,
-               &checkds->find);
+               adb, checkds->zone->loop, process_checkds_adb_event, checkds,
+               &checkds->ns, dns_rootname, 0, options, 0, NULL,
+               checkds->zone->view->dstport, 0, NULL, &checkds->find);
+       dns_adb_detach(&adb);
 
        /* Something failed? */
        if (result != ISC_R_SUCCESS) {
index 7c56c5f4a44e500ac0f2dd1e952620b012d2895f..05af2cadb44be1db855cc2be24961bac907d1b78 100644 (file)
@@ -234,7 +234,12 @@ ns_client_endrequest(ns_client_t *client) {
        if (client->view != NULL) {
 #ifdef ENABLE_AFL
                if (client->manager->sctx->fuzztype == isc_fuzz_resolver) {
-                       dns_adb_flush(client->view->adb);
+                       dns_adb_t *adb = NULL;
+                       dns_view_getadb(client->view, &adb);
+                       if (adb != NULL) {
+                               dns_adb_flush(adb);
+                               dns_adb_detach(&adb);
+                       }
                }
 #endif /* ifdef ENABLE_AFL */
                dns_view_detach(&client->view);