Merge branch '4021-tsan-error-view-adb-detached-too-early' into 'main'

Resolve "TSAN error: view->adb detached too early."

Closes #4021

See merge request isc-projects/bind9!8016
This commit is contained in:
Mark Andrews 2023-06-14 10:36:08 +00:00
commit 06bbe6a2db
8 changed files with 159 additions and 60 deletions

View file

@ -1,3 +1,5 @@
6195. [bug] Use rcu to reference view->adb. [GL #4021]
6194. [func] Change function 'find_zone_keys()' to look for signing
keys by looking for key files instead of a DNSKEY
RRset lookup. [GL #4141]

View 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);
}

View 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;
}

View 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

View 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);

View 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();
}

View 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) {

View 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);