From bfee4624036caf849721f7a986d36e736db6e16c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 7 Mar 2022 13:55:03 +0100 Subject: [PATCH 1/4] General cleanup of dns_rpz implementation Do a general cleanup of lib/dns/rpz.c style: * Removed deprecated and unused functions * Unified dns_rpz_zone_t naming to rpz * Unified dns_rpz_zones_t naming to rpzs * Add and use rpz_attach() and rpz_attach_rpzs() functions * Shuffled variables to be more local (cppcheck cleanup) (cherry picked from commit 840179a2473d65bfa193f4f8519e52517bbfa95e) --- lib/dns/include/dns/rpz.h | 8 - lib/dns/rpz.c | 496 ++++++++++++++++++-------------------- 2 files changed, 239 insertions(+), 265 deletions(-) diff --git a/lib/dns/include/dns/rpz.h b/lib/dns/include/dns/rpz.h index 33934df420..76b0200cf3 100644 --- a/lib/dns/include/dns/rpz.h +++ b/lib/dns/include/dns/rpz.h @@ -405,14 +405,6 @@ dns_rpz_attach_rpzs(dns_rpz_zones_t *source, dns_rpz_zones_t **target); void dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp); -isc_result_t -dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp, dns_rpz_zones_t *rpzs, - dns_rpz_num_t rpz_num) ISC_DEPRECATED; - -isc_result_t -dns_rpz_ready(dns_rpz_zones_t *rpzs, dns_rpz_zones_t **load_rpzsp, - dns_rpz_num_t rpz_num) ISC_DEPRECATED; - isc_result_t dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, const dns_name_t *name); diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 92eda66b9f..78f68673c5 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -172,30 +172,15 @@ struct dns_rpz_nm_data { dns_rpz_nm_zbits_t wild; }; +static void +rpz_attach(dns_rpz_zone_t *rpz, dns_rpz_zone_t **rpzp); static void rpz_detach(dns_rpz_zone_t **rpzp); static void -rpz_detach_rpzs(dns_rpz_zones_t **rpzsp); - -#if 0 -/* - * Catch a name while debugging. - */ +rpz_attach_rpzs(dns_rpz_zones_t *rpzs, dns_rpz_zones_t **rpzsp); static void -catch_name(const dns_name_t *src_name, const char *tgt, const char *str) { - dns_fixedname_t tgt_namef; - dns_name_t *tgt_name; - - tgt_name = dns_fixedname_initname(&tgt_namef); - dns_name_fromstring(tgt_name, tgt, DNS_NAME_DOWNCASE, NULL); - if (dns_name_equal(src_name, tgt_name)) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, - DNS_LOGMODULE_RBTDB, DNS_RPZ_ERROR_LEVEL, - "rpz hit failed: %s %s", str, tgt); - } -} -#endif /* if 0 */ +rpz_detach_rpzs(dns_rpz_zones_t **rpzsp); const char * dns_rpz_type2str(dns_rpz_type_t type) { @@ -370,13 +355,12 @@ make_nm_set(dns_rpz_nm_zbits_t *tgt_set, dns_rpz_num_t rpz_num, */ static void set_sum_pair(dns_rpz_cidr_node_t *cnode) { - dns_rpz_cidr_node_t *child; dns_rpz_addr_zbits_t sum; do { + dns_rpz_cidr_node_t *child = cnode->child[0]; sum = cnode->set; - child = cnode->child[0]; if (child != NULL) { sum.client_ip |= child->sum.client_ip; sum.ip |= child->sum.ip; @@ -640,13 +624,14 @@ new_node(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *ip, int i, words, wlen; node = isc_mem_get(rpzs->mctx, sizeof(*node)); - memset(node, 0, sizeof(*node)); + *node = (dns_rpz_cidr_node_t){ + .prefix = prefix, + }; if (child != NULL) { node->sum = child->sum; } - node->prefix = prefix; words = prefix / DNS_RPZ_CIDR_WORD_BITS; wlen = prefix % DNS_RPZ_CIDR_WORD_BITS; i = 0; @@ -667,12 +652,11 @@ new_node(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *ip, static void badname(int level, const dns_name_t *name, const char *str1, const char *str2) { - char namebuf[DNS_NAME_FORMATSIZE]; - /* * bin/tests/system/rpz/tests.sh looks for "invalid rpz". */ if (level < DNS_RPZ_DEBUG_QUIET && isc_log_wouldlog(dns_lctx, level)) { + char namebuf[DNS_NAME_FORMATSIZE]; dns_name_format(name, namebuf, sizeof(namebuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, DNS_LOGMODULE_RBTDB, level, @@ -698,12 +682,10 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, #ifndef INET6_ADDRSTRLEN #define INET6_ADDRSTRLEN 46 #endif /* ifndef INET6_ADDRSTRLEN */ - int w[DNS_RPZ_CIDR_WORDS * 2]; char str[1 + 8 + 1 + INET6_ADDRSTRLEN + 1]; isc_buffer_t buffer; isc_result_t result; - int best_first, best_len, cur_first, cur_len; - int i, n, len; + int len; if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) { len = snprintf(str, sizeof(str), "%u.%u.%u.%u.%u", @@ -715,16 +697,19 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, return (ISC_R_FAILURE); } } else { + int w[DNS_RPZ_CIDR_WORDS * 2]; + int best_first, best_len, cur_first, cur_len; + len = snprintf(str, sizeof(str), "%d", tgt_prefix); if (len < 0 || (size_t)len >= sizeof(str)) { return (ISC_R_FAILURE); } - for (i = 0; i < DNS_RPZ_CIDR_WORDS; i++) { - w[i * 2 + 1] = - ((tgt_ip->w[DNS_RPZ_CIDR_WORDS - 1 - i] >> 16) & + for (int n = 0; n < DNS_RPZ_CIDR_WORDS; n++) { + w[n * 2 + 1] = + ((tgt_ip->w[DNS_RPZ_CIDR_WORDS - 1 - n] >> 16) & 0xffff); - w[i * 2] = tgt_ip->w[DNS_RPZ_CIDR_WORDS - 1 - i] & + w[n * 2] = tgt_ip->w[DNS_RPZ_CIDR_WORDS - 1 - n] & 0xffff; } /* @@ -735,7 +720,7 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, best_len = 0; cur_first = -1; cur_len = 0; - for (n = 0; n <= 7; ++n) { + for (int n = 0; n <= 7; ++n) { if (w[n] != 0) { cur_len = 0; cur_first = -1; @@ -750,7 +735,9 @@ ip2name(const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, } } - for (n = 0; n <= 7; ++n) { + for (int n = 0; n <= 7; ++n) { + int i; + INSIST(len > 0 && (size_t)len < sizeof(str)); if (n == best_first) { i = snprintf(str + len, sizeof(str) - len, @@ -813,10 +800,10 @@ name2ipkey(int log_level, const dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t *tgt_prefix, dns_rpz_addr_zbits_t *new_set) { dns_rpz_zone_t *rpz; - char ip_str[DNS_NAME_FORMATSIZE], ip2_str[DNS_NAME_FORMATSIZE]; + char ip_str[DNS_NAME_FORMATSIZE]; dns_offsets_t ip_name_offsets; dns_fixedname_t ip_name2f; - dns_name_t ip_name, *ip_name2; + dns_name_t ip_name; const char *prefix_str, *cp, *end; char *cp2; int ip_labels; @@ -967,11 +954,12 @@ name2ipkey(int log_level, const dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, * Convert the address back to a canonical domain name * to ensure that the original name is in canonical form. */ - ip_name2 = dns_fixedname_initname(&ip_name2f); + dns_name_t *ip_name2 = dns_fixedname_initname(&ip_name2f); result = ip2name(tgt_ip, (dns_rpz_prefix_t)prefix_num, NULL, ip_name2); if (result != ISC_R_SUCCESS || !dns_name_equal(&ip_name, ip_name2)) { + char ip2_str[DNS_NAME_FORMATSIZE]; dns_name_format(ip_name2, ip2_str, sizeof(ip2_str)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, DNS_LOGMODULE_RBTDB, log_level, @@ -1131,7 +1119,6 @@ search(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_cidr_node_t *cur, *parent, *child, *new_parent, *sibling; dns_rpz_addr_zbits_t set; int cur_num, child_num; - dns_rpz_prefix_t dbit; isc_result_t find_result; set = *tgt_set; @@ -1141,6 +1128,7 @@ search(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *tgt_ip, parent = NULL; cur_num = 0; for (;;) { + dns_rpz_prefix_t dbit; if (cur == NULL) { /* * No child so we cannot go down. @@ -1449,67 +1437,71 @@ isc_result_t dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, char *rps_cstr, size_t rps_cstr_size, isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr) { - dns_rpz_zones_t *zones; + dns_rpz_zones_t *rpzs; isc_result_t result = ISC_R_SUCCESS; REQUIRE(rpzsp != NULL && *rpzsp == NULL); - zones = isc_mem_get(mctx, sizeof(*zones)); - memset(zones, 0, sizeof(*zones)); + rpzs = isc_mem_get(mctx, sizeof(*rpzs)); + *rpzs = (dns_rpz_zones_t){ + .rps_cstr = rps_cstr, + .rps_cstr_size = rps_cstr_size, + .timermgr = timermgr, + .taskmgr = taskmgr, + }; - isc_rwlock_init(&zones->search_lock, 0, 0); - isc_mutex_init(&zones->maint_lock); - isc_refcount_init(&zones->refs, 1); - isc_refcount_init(&zones->irefs, 1); + isc_rwlock_init(&rpzs->search_lock, 0, 0); + isc_mutex_init(&rpzs->maint_lock); + isc_refcount_init(&rpzs->refs, 1); + isc_refcount_init(&rpzs->irefs, 1); - zones->rps_cstr = rps_cstr; - zones->rps_cstr_size = rps_cstr_size; #ifdef USE_DNSRPS if (rps_cstr != NULL) { - result = dns_dnsrps_view_init(zones, rps_cstr); + result = dns_dnsrps_view_init(rpzs, rps_cstr); + if (result != ISC_R_SUCCESS) { + goto cleanup_rbt; + } } #else /* ifdef USE_DNSRPS */ - INSIST(!zones->p.dnsrps_enabled); + INSIST(!rpzs->p.dnsrps_enabled); #endif /* ifdef USE_DNSRPS */ - if (result == ISC_R_SUCCESS && !zones->p.dnsrps_enabled) { + if (!rpzs->p.dnsrps_enabled) { result = dns_rbt_create(mctx, rpz_node_deleter, mctx, - &zones->rbt); + &rpzs->rbt); } if (result != ISC_R_SUCCESS) { goto cleanup_rbt; } - result = isc_task_create(taskmgr, 0, &zones->updater); + result = isc_task_create(taskmgr, 0, &rpzs->updater); if (result != ISC_R_SUCCESS) { goto cleanup_task; } - isc_mem_attach(mctx, &zones->mctx); - zones->timermgr = timermgr; - zones->taskmgr = taskmgr; + isc_mem_attach(mctx, &rpzs->mctx); - *rpzsp = zones; + *rpzsp = rpzs; return (ISC_R_SUCCESS); cleanup_task: - dns_rbt_destroy(&zones->rbt); + dns_rbt_destroy(&rpzs->rbt); cleanup_rbt: - isc_refcount_decrementz(&zones->irefs); - isc_refcount_destroy(&zones->irefs); - isc_refcount_decrementz(&zones->refs); - isc_refcount_destroy(&zones->refs); - isc_mutex_destroy(&zones->maint_lock); - isc_rwlock_destroy(&zones->search_lock); - isc_mem_put(mctx, zones, sizeof(*zones)); + isc_refcount_decrementz(&rpzs->irefs); + isc_refcount_destroy(&rpzs->irefs); + isc_refcount_decrementz(&rpzs->refs); + isc_refcount_destroy(&rpzs->refs); + isc_mutex_destroy(&rpzs->maint_lock); + isc_rwlock_destroy(&rpzs->search_lock); + isc_mem_put(mctx, rpzs, sizeof(*rpzs)); return (result); } isc_result_t dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { - dns_rpz_zone_t *zone; + dns_rpz_zone_t *rpz; isc_result_t result; REQUIRE(rpzp != NULL && *rpzp == NULL); @@ -1518,15 +1510,16 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { return (ISC_R_NOSPACE); } - zone = isc_mem_get(rpzs->mctx, sizeof(*zone)); - - memset(zone, 0, sizeof(*zone)); - isc_refcount_init(&zone->refs, 1); + rpz = isc_mem_get(rpzs->mctx, sizeof(*rpz)); + *rpz = (dns_rpz_zone_t){ + .addsoa = true, + }; + isc_refcount_init(&rpz->refs, 1); result = isc_timer_create(rpzs->timermgr, isc_timertype_inactive, NULL, NULL, rpzs->updater, - dns_rpz_update_taskaction, zone, - &zone->updatetimer); + dns_rpz_update_taskaction, rpz, + &rpz->updatetimer); if (result != ISC_R_SUCCESS) { goto cleanup_timer; } @@ -1536,86 +1529,75 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { * simplifies update_from_db */ - isc_ht_init(&zone->nodes, rpzs->mctx, 1); + isc_ht_init(&rpz->nodes, rpzs->mctx, 1); - dns_name_init(&zone->origin, NULL); - dns_name_init(&zone->client_ip, NULL); - dns_name_init(&zone->ip, NULL); - dns_name_init(&zone->nsdname, NULL); - dns_name_init(&zone->nsip, NULL); - dns_name_init(&zone->passthru, NULL); - dns_name_init(&zone->drop, NULL); - dns_name_init(&zone->tcp_only, NULL); - dns_name_init(&zone->cname, NULL); + dns_name_init(&rpz->origin, NULL); + dns_name_init(&rpz->client_ip, NULL); + dns_name_init(&rpz->ip, NULL); + dns_name_init(&rpz->nsdname, NULL); + dns_name_init(&rpz->nsip, NULL); + dns_name_init(&rpz->passthru, NULL); + dns_name_init(&rpz->drop, NULL); + dns_name_init(&rpz->tcp_only, NULL); + dns_name_init(&rpz->cname, NULL); - isc_time_settoepoch(&zone->lastupdated); - zone->updatepending = false; - zone->updaterunning = false; - zone->db = NULL; - zone->dbversion = NULL; - zone->updb = NULL; - zone->updbversion = NULL; - zone->updbit = NULL; - isc_refcount_increment(&rpzs->irefs); - zone->rpzs = rpzs; - zone->db_registered = false; - zone->addsoa = true; - ISC_EVENT_INIT(&zone->updateevent, sizeof(zone->updateevent), 0, NULL, - 0, NULL, NULL, NULL, NULL, NULL); + isc_time_settoepoch(&rpz->lastupdated); - zone->num = rpzs->p.num_zones++; - rpzs->zones[zone->num] = zone; + rpz_attach_rpzs(rpzs, &rpz->rpzs); - *rpzp = zone; + ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, NULL, 0, + NULL, NULL, NULL, NULL, NULL); + + rpz->num = rpzs->p.num_zones++; + rpzs->zones[rpz->num] = rpz; + + *rpzp = rpz; return (ISC_R_SUCCESS); cleanup_timer: - isc_refcount_decrementz(&zone->refs); - isc_refcount_destroy(&zone->refs); - - isc_mem_put(rpzs->mctx, zone, sizeof(*zone)); + isc_mem_put(rpzs->mctx, rpz, sizeof(*rpz)); return (result); } isc_result_t dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { - dns_rpz_zone_t *zone = (dns_rpz_zone_t *)fn_arg; + dns_rpz_zone_t *rpz = (dns_rpz_zone_t *)fn_arg; isc_time_t now; uint64_t tdiff; isc_result_t result = ISC_R_SUCCESS; char dname[DNS_NAME_FORMATSIZE]; REQUIRE(DNS_DB_VALID(db)); - REQUIRE(zone != NULL); + REQUIRE(rpz != NULL); - LOCK(&zone->rpzs->maint_lock); + LOCK(&rpz->rpzs->maint_lock); /* New zone came as AXFR */ - if (zone->db != NULL && zone->db != db) { + if (rpz->db != NULL && rpz->db != db) { /* We need to clean up the old DB */ - if (zone->dbversion != NULL) { - dns_db_closeversion(zone->db, &zone->dbversion, false); + if (rpz->dbversion != NULL) { + dns_db_closeversion(rpz->db, &rpz->dbversion, false); } - dns_db_updatenotify_unregister(zone->db, - dns_rpz_dbupdate_callback, zone); - dns_db_detach(&zone->db); + dns_db_updatenotify_unregister(rpz->db, + dns_rpz_dbupdate_callback, rpz); + dns_db_detach(&rpz->db); } - if (zone->db == NULL) { - RUNTIME_CHECK(zone->dbversion == NULL); - dns_db_attach(db, &zone->db); + if (rpz->db == NULL) { + RUNTIME_CHECK(rpz->dbversion == NULL); + dns_db_attach(db, &rpz->db); } - if (!zone->updatepending && !zone->updaterunning) { - zone->updatepending = true; + if (!rpz->updatepending && !rpz->updaterunning) { + rpz->updatepending = true; isc_time_now(&now); - tdiff = isc_time_microdiff(&now, &zone->lastupdated) / 1000000; - if (tdiff < zone->min_update_interval) { - uint64_t defer = zone->min_update_interval - tdiff; + tdiff = isc_time_microdiff(&now, &rpz->lastupdated) / 1000000; + if (tdiff < rpz->min_update_interval) { + uint64_t defer = rpz->min_update_interval - tdiff; isc_interval_t interval; - dns_name_format(&zone->origin, dname, + dns_name_format(&rpz->origin, dname, DNS_NAME_FORMATSIZE); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_INFO, @@ -1624,8 +1606,8 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { "%" PRIu64 " seconds", dname, defer); isc_interval_set(&interval, (unsigned int)defer, 0); - dns_db_currentversion(zone->db, &zone->dbversion); - result = isc_timer_reset(zone->updatetimer, + dns_db_currentversion(rpz->db, &rpz->dbversion); + result = isc_timer_reset(rpz->updatetimer, isc_timertype_once, NULL, &interval, true); if (result != ISC_R_SUCCESS) { @@ -1634,31 +1616,31 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { } else { isc_event_t *event; - dns_db_currentversion(zone->db, &zone->dbversion); - INSIST(!ISC_LINK_LINKED(&zone->updateevent, ev_link)); - ISC_EVENT_INIT(&zone->updateevent, - sizeof(zone->updateevent), 0, NULL, + dns_db_currentversion(rpz->db, &rpz->dbversion); + INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); + ISC_EVENT_INIT(&rpz->updateevent, + sizeof(rpz->updateevent), 0, NULL, DNS_EVENT_RPZUPDATED, - dns_rpz_update_taskaction, zone, zone, + dns_rpz_update_taskaction, rpz, rpz, NULL, NULL); - event = &zone->updateevent; - isc_task_send(zone->rpzs->updater, &event); + event = &rpz->updateevent; + isc_task_send(rpz->rpzs->updater, &event); } } else { - zone->updatepending = true; - dns_name_format(&zone->origin, dname, DNS_NAME_FORMATSIZE); + rpz->updatepending = true; + dns_name_format(&rpz->origin, dname, DNS_NAME_FORMATSIZE); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), "rpz: %s: update already queued or running", dname); - if (zone->dbversion != NULL) { - dns_db_closeversion(zone->db, &zone->dbversion, false); + if (rpz->dbversion != NULL) { + dns_db_closeversion(rpz->db, &rpz->dbversion, false); } - dns_db_currentversion(zone->db, &zone->dbversion); + dns_db_currentversion(rpz->db, &rpz->dbversion); } cleanup: - UNLOCK(&zone->rpzs->maint_lock); + UNLOCK(&rpz->rpzs->maint_lock); return (result); } @@ -1666,24 +1648,24 @@ cleanup: static void dns_rpz_update_taskaction(isc_task_t *task, isc_event_t *event) { isc_result_t result; - dns_rpz_zone_t *zone; + dns_rpz_zone_t *rpz; REQUIRE(event != NULL); REQUIRE(event->ev_arg != NULL); UNUSED(task); - zone = (dns_rpz_zone_t *)event->ev_arg; + rpz = (dns_rpz_zone_t *)event->ev_arg; isc_event_free(&event); - LOCK(&zone->rpzs->maint_lock); - zone->updatepending = false; - zone->updaterunning = true; - dns_rpz_update_from_db(zone); - result = isc_timer_reset(zone->updatetimer, isc_timertype_inactive, - NULL, NULL, true); + LOCK(&rpz->rpzs->maint_lock); + rpz->updatepending = false; + rpz->updaterunning = true; + dns_rpz_update_from_db(rpz); + result = isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, + NULL, true); RUNTIME_CHECK(result == ISC_R_SUCCESS); - result = isc_time_now(&zone->lastupdated); + result = isc_time_now(&rpz->lastupdated); RUNTIME_CHECK(result == ISC_R_SUCCESS); - UNLOCK(&zone->rpzs->maint_lock); + UNLOCK(&rpz->rpzs->maint_lock); } static isc_result_t @@ -2082,7 +2064,8 @@ dns_rpz_update_from_db(dns_rpz_zone_t *rpz) { REQUIRE(rpz->updbit == NULL); REQUIRE(rpz->newnodes == NULL); - isc_refcount_increment(&rpz->refs); + rpz_attach(rpz, &(dns_rpz_zone_t *){ NULL }); + dns_db_attach(rpz->db, &rpz->updb); rpz->updbversion = rpz->dbversion; rpz->dbversion = NULL; @@ -2145,6 +2128,84 @@ cidr_free(dns_rpz_zones_t *rpzs) { } } +static void +rpz_attach(dns_rpz_zone_t *rpz, dns_rpz_zone_t **rpzp) { + REQUIRE(rpz != NULL); + REQUIRE(rpzp != NULL && *rpzp == NULL); + + isc_refcount_increment(&rpz->refs); + *rpzp = rpz; +} + +static void +rpz_destroy(dns_rpz_zone_t *rpz) { + dns_rpz_zones_t *rpzs = rpz->rpzs; + rpz->rpzs = NULL; + + isc_refcount_destroy(&rpz->refs); + + if (dns_name_dynamic(&rpz->origin)) { + dns_name_free(&rpz->origin, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->client_ip)) { + dns_name_free(&rpz->client_ip, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->ip)) { + dns_name_free(&rpz->ip, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->nsdname)) { + dns_name_free(&rpz->nsdname, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->nsip)) { + dns_name_free(&rpz->nsip, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->passthru)) { + dns_name_free(&rpz->passthru, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->drop)) { + dns_name_free(&rpz->drop, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->tcp_only)) { + dns_name_free(&rpz->tcp_only, rpzs->mctx); + } + if (dns_name_dynamic(&rpz->cname)) { + dns_name_free(&rpz->cname, rpzs->mctx); + } + if (rpz->db != NULL) { + if (rpz->dbversion != NULL) { + dns_db_closeversion(rpz->db, &rpz->dbversion, false); + } + dns_db_updatenotify_unregister(rpz->db, + dns_rpz_dbupdate_callback, rpz); + dns_db_detach(&rpz->db); + } + if (rpz->updaterunning) { + isc_task_purgeevent(rpzs->updater, &rpz->updateevent); + if (rpz->updbit != NULL) { + dns_dbiterator_destroy(&rpz->updbit); + } + if (rpz->newnodes != NULL) { + isc_ht_destroy(&rpz->newnodes); + } + if (rpz->updb != NULL) { + if (rpz->updbversion != NULL) { + dns_db_closeversion(rpz->updb, + &rpz->updbversion, false); + } + dns_db_detach(&rpz->updb); + } + } + + isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, NULL, + true); + isc_timer_detach(&rpz->updatetimer); + + isc_ht_destroy(&rpz->nodes); + + isc_mem_put(rpzs->mctx, rpz, sizeof(*rpz)); + rpz_detach_rpzs(&rpzs); +} + /* * Discard a response policy zone blob * before discarding the overall rpz structure. @@ -2152,7 +2213,6 @@ cidr_free(dns_rpz_zones_t *rpzs) { static void rpz_detach(dns_rpz_zone_t **rpzp) { dns_rpz_zone_t *rpz; - dns_rpz_zones_t *rpzs; REQUIRE(rpzp != NULL && *rpzp != NULL); @@ -2160,73 +2220,7 @@ rpz_detach(dns_rpz_zone_t **rpzp) { *rpzp = NULL; if (isc_refcount_decrement(&rpz->refs) == 1) { - isc_refcount_destroy(&rpz->refs); - - rpzs = rpz->rpzs; - rpz->rpzs = NULL; - - if (dns_name_dynamic(&rpz->origin)) { - dns_name_free(&rpz->origin, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->client_ip)) { - dns_name_free(&rpz->client_ip, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->ip)) { - dns_name_free(&rpz->ip, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->nsdname)) { - dns_name_free(&rpz->nsdname, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->nsip)) { - dns_name_free(&rpz->nsip, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->passthru)) { - dns_name_free(&rpz->passthru, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->drop)) { - dns_name_free(&rpz->drop, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->tcp_only)) { - dns_name_free(&rpz->tcp_only, rpzs->mctx); - } - if (dns_name_dynamic(&rpz->cname)) { - dns_name_free(&rpz->cname, rpzs->mctx); - } - if (rpz->db != NULL) { - if (rpz->dbversion != NULL) { - dns_db_closeversion(rpz->db, &rpz->dbversion, - false); - } - dns_db_updatenotify_unregister( - rpz->db, dns_rpz_dbupdate_callback, rpz); - dns_db_detach(&rpz->db); - } - if (rpz->updaterunning) { - isc_task_purgeevent(rpzs->updater, &rpz->updateevent); - if (rpz->updbit != NULL) { - dns_dbiterator_destroy(&rpz->updbit); - } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); - } - if (rpz->updb != NULL) { - if (rpz->updbversion != NULL) { - dns_db_closeversion(rpz->updb, - &rpz->updbversion, - false); - } - dns_db_detach(&rpz->updb); - } - } - - isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, - NULL, true); - isc_timer_detach(&rpz->updatetimer); - - isc_ht_destroy(&rpz->nodes); - - isc_mem_put(rpzs->mctx, rpz, sizeof(*rpz)); - rpz_detach_rpzs(&rpzs); + rpz_destroy(rpz); } } @@ -2265,6 +2259,36 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) { } } +static void +rpz_attach_rpzs(dns_rpz_zones_t *rpzs, dns_rpz_zones_t **rpzsp) { + REQUIRE(rpzs != NULL); + REQUIRE(rpzsp != NULL && *rpzsp == NULL); + + isc_refcount_increment(&rpzs->irefs); + + *rpzsp = rpzs; +} + +static void +rpz_destroy_rpzs(dns_rpz_zones_t *rpzs) { + if (rpzs->rps_cstr_size != 0) { +#ifdef USE_DNSRPS + librpz->client_detach(&rpzs->rps_client); +#endif /* ifdef USE_DNSRPS */ + isc_mem_put(rpzs->mctx, rpzs->rps_cstr, rpzs->rps_cstr_size); + } + + cidr_free(rpzs); + if (rpzs->rbt != NULL) { + dns_rbt_destroy(&rpzs->rbt); + } + isc_task_destroy(&rpzs->updater); + isc_mutex_destroy(&rpzs->maint_lock); + isc_rwlock_destroy(&rpzs->search_lock); + isc_refcount_destroy(&rpzs->refs); + isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs)); +} + static void rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) { REQUIRE(rpzsp != NULL && *rpzsp != NULL); @@ -2272,52 +2296,10 @@ rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) { *rpzsp = NULL; if (isc_refcount_decrement(&rpzs->irefs) == 1) { - if (rpzs->rps_cstr_size != 0) { -#ifdef USE_DNSRPS - librpz->client_detach(&rpzs->rps_client); -#endif /* ifdef USE_DNSRPS */ - isc_mem_put(rpzs->mctx, rpzs->rps_cstr, - rpzs->rps_cstr_size); - } - - cidr_free(rpzs); - if (rpzs->rbt != NULL) { - dns_rbt_destroy(&rpzs->rbt); - } - isc_task_destroy(&rpzs->updater); - isc_mutex_destroy(&rpzs->maint_lock); - isc_rwlock_destroy(&rpzs->search_lock); - isc_refcount_destroy(&rpzs->refs); - isc_mem_putanddetach(&rpzs->mctx, rpzs, sizeof(*rpzs)); + rpz_destroy_rpzs(rpzs); } } -/* - * Deprecated and removed. - */ -isc_result_t -dns_rpz_beginload(dns_rpz_zones_t **load_rpzsp, dns_rpz_zones_t *rpzs, - dns_rpz_num_t rpz_num) { - UNUSED(load_rpzsp); - UNUSED(rpzs); - UNUSED(rpz_num); - - return (ISC_R_NOTIMPLEMENTED); -} - -/* - * Deprecated and removed. - */ -isc_result_t -dns_rpz_ready(dns_rpz_zones_t *rpzs, dns_rpz_zones_t **load_rpzsp, - dns_rpz_num_t rpz_num) { - UNUSED(rpzs); - UNUSED(load_rpzsp); - UNUSED(rpz_num); - - return (ISC_R_NOTIMPLEMENTED); -} - /* * Add an IP address to the radix tree or a name to the summary database. */ From f4cba0784eab4a9a8b5216e12f72e01e0762f681 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 8 Mar 2022 17:13:26 +0100 Subject: [PATCH 2/4] Refactor the dns_rpz_add/delete to use local rpz copy Previously dns_rpz_add() were passed dns_rpz_zones_t and index to .zones array. Because we actually attach to dns_rpz_zone_t, we should be using the local pointer instead of passing the index and "finding" the dns_rpz_zone_t again. Additionally, dns_rpz_add() and dns_rpz_delete() were used only inside rpz.c, so make them static. (cherry picked from commit b6e885c97ff5e80d9108fb53eed28cf11aadbb86) --- lib/dns/include/dns/rpz.h | 8 -- lib/dns/rpz.c | 224 ++++++++++++++++++++------------------ 2 files changed, 119 insertions(+), 113 deletions(-) diff --git a/lib/dns/include/dns/rpz.h b/lib/dns/include/dns/rpz.h index 76b0200cf3..cb4be697e4 100644 --- a/lib/dns/include/dns/rpz.h +++ b/lib/dns/include/dns/rpz.h @@ -405,14 +405,6 @@ dns_rpz_attach_rpzs(dns_rpz_zones_t *source, dns_rpz_zones_t **target); void dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp); -isc_result_t -dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - const dns_name_t *name); - -void -dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - const dns_name_t *name); - dns_rpz_num_t dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, dns_rpz_zbits_t zbits, const isc_netaddr_t *netaddr, diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 78f68673c5..4bf4650c92 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -94,7 +94,7 @@ #define DNS_RPZ_QUANTUM 1024 static void -dns_rpz_update_from_db(dns_rpz_zone_t *rpz); +update_from_db(dns_rpz_zone_t *rpz); static void dns_rpz_update_taskaction(isc_task_t *task, isc_event_t *event); @@ -172,6 +172,11 @@ struct dns_rpz_nm_data { dns_rpz_nm_zbits_t wild; }; +static isc_result_t +rpz_add(dns_rpz_zone_t *rpz, const dns_name_t *src_name); +static void +rpz_del(dns_rpz_zone_t *rpz, const dns_name_t *src_name); + static void rpz_attach(dns_rpz_zone_t *rpz, dns_rpz_zone_t **rpzp); static void @@ -233,7 +238,7 @@ dns_rpz_str2policy(const char *str) { const char * dns_rpz_policy2str(dns_rpz_policy_t policy) { - const char *str; + const char *str = NULL; switch (policy) { case DNS_RPZ_POLICY_PASSTHRU: @@ -554,9 +559,9 @@ set: } static void -adj_trigger_cnt(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - dns_rpz_type_t rpz_type, const dns_rpz_cidr_key_t *tgt_ip, - dns_rpz_prefix_t tgt_prefix, bool inc) { +adj_trigger_cnt(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, + const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, + bool inc) { dns_rpz_trigger_counter_t *cnt = NULL; dns_rpz_zbits_t *have = NULL; @@ -564,39 +569,39 @@ adj_trigger_cnt(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, case DNS_RPZ_TYPE_CLIENT_IP: REQUIRE(tgt_ip != NULL); if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) { - cnt = &rpzs->triggers[rpz_num].client_ipv4; - have = &rpzs->have.client_ipv4; + cnt = &rpz->rpzs->triggers[rpz->num].client_ipv4; + have = &rpz->rpzs->have.client_ipv4; } else { - cnt = &rpzs->triggers[rpz_num].client_ipv6; - have = &rpzs->have.client_ipv6; + cnt = &rpz->rpzs->triggers[rpz->num].client_ipv6; + have = &rpz->rpzs->have.client_ipv6; } break; case DNS_RPZ_TYPE_QNAME: - cnt = &rpzs->triggers[rpz_num].qname; - have = &rpzs->have.qname; + cnt = &rpz->rpzs->triggers[rpz->num].qname; + have = &rpz->rpzs->have.qname; break; case DNS_RPZ_TYPE_IP: REQUIRE(tgt_ip != NULL); if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) { - cnt = &rpzs->triggers[rpz_num].ipv4; - have = &rpzs->have.ipv4; + cnt = &rpz->rpzs->triggers[rpz->num].ipv4; + have = &rpz->rpzs->have.ipv4; } else { - cnt = &rpzs->triggers[rpz_num].ipv6; - have = &rpzs->have.ipv6; + cnt = &rpz->rpzs->triggers[rpz->num].ipv6; + have = &rpz->rpzs->have.ipv6; } break; case DNS_RPZ_TYPE_NSDNAME: - cnt = &rpzs->triggers[rpz_num].nsdname; - have = &rpzs->have.nsdname; + cnt = &rpz->rpzs->triggers[rpz->num].nsdname; + have = &rpz->rpzs->have.nsdname; break; case DNS_RPZ_TYPE_NSIP: REQUIRE(tgt_ip != NULL); if (KEY_IS_IPV4(tgt_prefix, tgt_ip)) { - cnt = &rpzs->triggers[rpz_num].nsipv4; - have = &rpzs->have.nsipv4; + cnt = &rpz->rpzs->triggers[rpz->num].nsipv4; + have = &rpz->rpzs->have.nsipv4; } else { - cnt = &rpzs->triggers[rpz_num].nsipv6; - have = &rpzs->have.nsipv6; + cnt = &rpz->rpzs->triggers[rpz->num].nsipv6; + have = &rpz->rpzs->have.nsipv6; } break; default: @@ -605,14 +610,14 @@ adj_trigger_cnt(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, if (inc) { if (++*cnt == 1U) { - *have |= DNS_RPZ_ZBIT(rpz_num); - fix_qname_skip_recurse(rpzs); + *have |= DNS_RPZ_ZBIT(rpz->num); + fix_qname_skip_recurse(rpz->rpzs); } } else { REQUIRE(*cnt != 0U); if (--*cnt == 0U) { - *have &= ~DNS_RPZ_ZBIT(rpz_num); - fix_qname_skip_recurse(rpzs); + *have &= ~DNS_RPZ_ZBIT(rpz->num); + fix_qname_skip_recurse(rpz->rpzs); } } } @@ -620,7 +625,7 @@ adj_trigger_cnt(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, static dns_rpz_cidr_node_t * new_node(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *ip, dns_rpz_prefix_t prefix, const dns_rpz_cidr_node_t *child) { - dns_rpz_cidr_node_t *node; + dns_rpz_cidr_node_t *node = NULL; int i, words, wlen; node = isc_mem_get(rpzs->mctx, sizeof(*node)); @@ -795,16 +800,14 @@ type_from_name(const dns_rpz_zones_t *rpzs, dns_rpz_zone_t *rpz, * data. */ static isc_result_t -name2ipkey(int log_level, const dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - dns_rpz_type_t rpz_type, const dns_name_t *src_name, - dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t *tgt_prefix, - dns_rpz_addr_zbits_t *new_set) { - dns_rpz_zone_t *rpz; +name2ipkey(int log_level, dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, + const dns_name_t *src_name, dns_rpz_cidr_key_t *tgt_ip, + dns_rpz_prefix_t *tgt_prefix, dns_rpz_addr_zbits_t *new_set) { char ip_str[DNS_NAME_FORMATSIZE]; dns_offsets_t ip_name_offsets; dns_fixedname_t ip_name2f; dns_name_t ip_name; - const char *prefix_str, *cp, *end; + const char *prefix_str = NULL, *cp = NULL, *end = NULL; char *cp2; int ip_labels; dns_rpz_prefix_t prefix; @@ -812,11 +815,10 @@ name2ipkey(int log_level, const dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, isc_result_t result; int i; - REQUIRE(rpzs != NULL && rpz_num < rpzs->p.num_zones); - rpz = rpzs->zones[rpz_num]; REQUIRE(rpz != NULL); + REQUIRE(rpz->rpzs != NULL && rpz->num < rpz->rpzs->p.num_zones); - make_addr_set(new_set, DNS_RPZ_ZBIT(rpz_num), rpz_type); + make_addr_set(new_set, DNS_RPZ_ZBIT(rpz->num), rpz_type); ip_labels = dns_name_countlabels(src_name); if (rpz_type == DNS_RPZ_TYPE_QNAME) { @@ -977,17 +979,15 @@ name2ipkey(int log_level, const dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, * or QNAME data. */ static void -name2data(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, +name2data(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, const dns_name_t *src_name, dns_name_t *trig_name, dns_rpz_nm_data_t *new_data) { - dns_rpz_zone_t *rpz; dns_offsets_t tmp_name_offsets; dns_name_t tmp_name; unsigned int prefix_len, n; - REQUIRE(rpzs != NULL && rpz_num < rpzs->p.num_zones); - rpz = rpzs->zones[rpz_num]; REQUIRE(rpz != NULL); + REQUIRE(rpz->rpzs != NULL && rpz->num < rpz->rpzs->p.num_zones); /* * Handle wildcards by putting only the parent into the @@ -997,10 +997,10 @@ name2data(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, if (dns_name_iswildcard(src_name)) { prefix_len = 1; memset(&new_data->set, 0, sizeof(new_data->set)); - make_nm_set(&new_data->wild, rpz_num, rpz_type); + make_nm_set(&new_data->wild, rpz->num, rpz_type); } else { prefix_len = 0; - make_nm_set(&new_data->set, rpz_num, rpz_type); + make_nm_set(&new_data->set, rpz->num, rpz_type); memset(&new_data->wild, 0, sizeof(new_data->wild)); } @@ -1116,7 +1116,8 @@ static isc_result_t search(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *tgt_ip, dns_rpz_prefix_t tgt_prefix, const dns_rpz_addr_zbits_t *tgt_set, bool create, dns_rpz_cidr_node_t **found) { - dns_rpz_cidr_node_t *cur, *parent, *child, *new_parent, *sibling; + dns_rpz_cidr_node_t *cur = NULL, *parent = NULL, *child = NULL; + dns_rpz_cidr_node_t *new_parent = NULL, *sibling = NULL; dns_rpz_addr_zbits_t set; int cur_num, child_num; isc_result_t find_result; @@ -1305,16 +1306,16 @@ search(dns_rpz_zones_t *rpzs, const dns_rpz_cidr_key_t *tgt_ip, * Add an IP address to the radix tree. */ static isc_result_t -add_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, +add_cidr(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, const dns_name_t *src_name) { dns_rpz_cidr_key_t tgt_ip; dns_rpz_prefix_t tgt_prefix; dns_rpz_addr_zbits_t set; - dns_rpz_cidr_node_t *found; + dns_rpz_cidr_node_t *found = NULL; isc_result_t result; - result = name2ipkey(DNS_RPZ_ERROR_LEVEL, rpzs, rpz_num, rpz_type, - src_name, &tgt_ip, &tgt_prefix, &set); + result = name2ipkey(DNS_RPZ_ERROR_LEVEL, rpz, rpz_type, src_name, + &tgt_ip, &tgt_prefix, &set); /* * Log complaints about bad owner names but let the zone load. */ @@ -1322,7 +1323,7 @@ add_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, return (ISC_R_SUCCESS); } - result = search(rpzs, &tgt_ip, tgt_prefix, &set, true, &found); + result = search(rpz->rpzs, &tgt_ip, tgt_prefix, &set, true, &found); if (result != ISC_R_SUCCESS) { char namebuf[DNS_NAME_FORMATSIZE]; @@ -1345,15 +1346,15 @@ add_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, return (result); } - adj_trigger_cnt(rpzs, rpz_num, rpz_type, &tgt_ip, tgt_prefix, true); + adj_trigger_cnt(rpz, rpz_type, &tgt_ip, tgt_prefix, true); return (result); } static isc_result_t add_nm(dns_rpz_zones_t *rpzs, dns_name_t *trig_name, const dns_rpz_nm_data_t *new_data) { - dns_rbtnode_t *nmnode; - dns_rpz_nm_data_t *nm_data; + dns_rbtnode_t *nmnode = NULL; + dns_rpz_nm_data_t *nm_data = NULL; isc_result_t result; nmnode = NULL; @@ -1392,11 +1393,11 @@ add_nm(dns_rpz_zones_t *rpzs, dns_name_t *trig_name, } static isc_result_t -add_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, +add_name(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, const dns_name_t *src_name) { dns_rpz_nm_data_t new_data; dns_fixedname_t trig_namef; - dns_name_t *trig_name; + dns_name_t *trig_name = NULL; isc_result_t result; /* @@ -1405,9 +1406,9 @@ add_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, */ trig_name = dns_fixedname_initname(&trig_namef); - name2data(rpzs, rpz_num, rpz_type, src_name, trig_name, &new_data); + name2data(rpz, rpz_type, src_name, trig_name, &new_data); - result = add_nm(rpzs, trig_name, &new_data); + result = add_nm(rpz->rpzs, trig_name, &new_data); /* * Do not worry if the node already exists, @@ -1417,7 +1418,7 @@ add_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, return (ISC_R_SUCCESS); } if (result == ISC_R_SUCCESS) { - adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, true); + adj_trigger_cnt(rpz, rpz_type, NULL, 0, true); } return (result); } @@ -1437,7 +1438,7 @@ isc_result_t dns_rpz_new_zones(dns_rpz_zones_t **rpzsp, char *rps_cstr, size_t rps_cstr_size, isc_mem_t *mctx, isc_taskmgr_t *taskmgr, isc_timermgr_t *timermgr) { - dns_rpz_zones_t *rpzs; + dns_rpz_zones_t *rpzs = NULL; isc_result_t result = ISC_R_SUCCESS; REQUIRE(rpzsp != NULL && *rpzsp == NULL); @@ -1501,7 +1502,7 @@ cleanup_rbt: isc_result_t dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { - dns_rpz_zone_t *rpz; + dns_rpz_zone_t *rpz = NULL; isc_result_t result; REQUIRE(rpzp != NULL && *rpzp == NULL); @@ -1614,7 +1615,7 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { goto cleanup; } } else { - isc_event_t *event; + isc_event_t *event = NULL; dns_db_currentversion(rpz->db, &rpz->dbversion); INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); @@ -1648,7 +1649,7 @@ cleanup: static void dns_rpz_update_taskaction(isc_task_t *task, isc_event_t *event) { isc_result_t result; - dns_rpz_zone_t *rpz; + dns_rpz_zone_t *rpz = NULL; REQUIRE(event != NULL); REQUIRE(event->ev_arg != NULL); @@ -1659,7 +1660,10 @@ dns_rpz_update_taskaction(isc_task_t *task, isc_event_t *event) { LOCK(&rpz->rpzs->maint_lock); rpz->updatepending = false; rpz->updaterunning = true; - dns_rpz_update_from_db(rpz); + rpz->updateresult = ISC_R_UNSET; + + update_from_db(rpz); + result = isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, NULL, true); RUNTIME_CHECK(result == ISC_R_SUCCESS); @@ -1827,7 +1831,7 @@ cleanup_quantum(isc_task_t *task, isc_event_t *event) { region.base = key; region.length = (unsigned int)keysize; dns_name_fromregion(name, ®ion); - dns_rpz_delete(rpz->rpzs, rpz->num, name); + rpz_del(rpz, name); } if (result == ISC_R_SUCCESS) { @@ -1977,7 +1981,7 @@ update_quantum(isc_task_t *task, isc_event_t *event) { if (result == ISC_R_SUCCESS) { isc_ht_delete(rpz->nodes, name->ndata, name->length); } else { /* not found */ - result = dns_rpz_add(rpz->rpzs, rpz->num, name); + result = rpz_add(rpz, name); if (result != ISC_R_SUCCESS) { dns_name_format(name, namebuf, sizeof(namebuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -2053,9 +2057,8 @@ cleanup: } static void -dns_rpz_update_from_db(dns_rpz_zone_t *rpz) { - isc_result_t result; - isc_event_t *event; +update_from_db(dns_rpz_zone_t *rpz) { + dns_rpz_zone_t *rpz_zone = NULL; REQUIRE(rpz != NULL); REQUIRE(DNS_DB_VALID(rpz->db)); @@ -2100,7 +2103,7 @@ cleanup: */ static void cidr_free(dns_rpz_zones_t *rpzs) { - dns_rpz_cidr_node_t *cur, *child, *parent; + dns_rpz_cidr_node_t *cur = NULL, *child = NULL, *parent = NULL; cur = rpzs->cidr; while (cur != NULL) { @@ -2212,7 +2215,7 @@ rpz_destroy(dns_rpz_zone_t *rpz) { */ static void rpz_detach(dns_rpz_zone_t **rpzp) { - dns_rpz_zone_t *rpz; + dns_rpz_zone_t *rpz = NULL; REQUIRE(rpzp != NULL && *rpzp != NULL); @@ -2303,16 +2306,20 @@ rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) { /* * Add an IP address to the radix tree or a name to the summary database. */ -isc_result_t -dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - const dns_name_t *src_name) { - dns_rpz_zone_t *rpz; +static isc_result_t +rpz_add(dns_rpz_zone_t *rpz, const dns_name_t *src_name) { dns_rpz_type_t rpz_type; isc_result_t result = ISC_R_FAILURE; + dns_rpz_zones_t *rpzs = NULL; + dns_rpz_num_t rpz_num; + + REQUIRE(rpz != NULL); + + rpzs = rpz->rpzs; + rpz_num = rpz->num; REQUIRE(rpzs != NULL && rpz_num < rpzs->p.num_zones); - rpz = rpzs->zones[rpz_num]; - REQUIRE(rpz != NULL); + RWLOCK(&rpzs->search_lock, isc_rwlocktype_write); rpz_type = type_from_name(rpzs, rpz, src_name); @@ -2320,12 +2327,12 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, switch (rpz_type) { case DNS_RPZ_TYPE_QNAME: case DNS_RPZ_TYPE_NSDNAME: - result = add_name(rpzs, rpz_num, rpz_type, src_name); + result = add_name(rpz, rpz_type, src_name); break; case DNS_RPZ_TYPE_CLIENT_IP: case DNS_RPZ_TYPE_IP: case DNS_RPZ_TYPE_NSIP: - result = add_cidr(rpzs, rpz_num, rpz_type, src_name); + result = add_cidr(rpz, rpz_type, src_name); break; case DNS_RPZ_TYPE_BAD: break; @@ -2339,26 +2346,26 @@ dns_rpz_add(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, * Remove an IP address from the radix tree. */ static void -del_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, +del_cidr(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, const dns_name_t *src_name) { isc_result_t result; dns_rpz_cidr_key_t tgt_ip; dns_rpz_prefix_t tgt_prefix; dns_rpz_addr_zbits_t tgt_set; - dns_rpz_cidr_node_t *tgt, *parent, *child; + dns_rpz_cidr_node_t *tgt = NULL, *parent = NULL, *child = NULL; /* * Do not worry about invalid rpz IP address names. If we * are here, then something relevant was added and so was * valid. Invalid names here are usually internal RBTDB nodes. */ - result = name2ipkey(DNS_RPZ_DEBUG_QUIET, rpzs, rpz_num, rpz_type, - src_name, &tgt_ip, &tgt_prefix, &tgt_set); + result = name2ipkey(DNS_RPZ_DEBUG_QUIET, rpz, rpz_type, src_name, + &tgt_ip, &tgt_prefix, &tgt_set); if (result != ISC_R_SUCCESS) { return; } - result = search(rpzs, &tgt_ip, tgt_prefix, &tgt_set, false, &tgt); + result = search(rpz->rpzs, &tgt_ip, tgt_prefix, &tgt_set, false, &tgt); if (result != ISC_R_SUCCESS) { INSIST(result == ISC_R_NOTFOUND || result == DNS_R_PARTIALMATCH); @@ -2383,7 +2390,7 @@ del_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, tgt->set.nsip &= ~tgt_set.nsip; set_sum_pair(tgt); - adj_trigger_cnt(rpzs, rpz_num, rpz_type, &tgt_ip, tgt_prefix, false); + adj_trigger_cnt(rpz, rpz_type, &tgt_ip, tgt_prefix, false); /* * We might need to delete 2 nodes. @@ -2391,7 +2398,8 @@ del_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, do { /* * The node is now useless if it has no data of its own - * and 0 or 1 children. We are finished if it is not useless. + * and 0 or 1 children. We are finished if it is not + * useless. */ if ((child = tgt->child[0]) != NULL) { if (tgt->child[1] != NULL) { @@ -2411,30 +2419,32 @@ del_cidr(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, */ parent = tgt->parent; if (parent == NULL) { - rpzs->cidr = child; + rpz->rpzs->cidr = child; } else { parent->child[parent->child[1] == tgt] = child; } + /* * If the child exists fix up its parent pointer. */ if (child != NULL) { child->parent = parent; } - isc_mem_put(rpzs->mctx, tgt, sizeof(*tgt)); + isc_mem_put(rpz->rpzs->mctx, tgt, sizeof(*tgt)); tgt = parent; } while (tgt != NULL); } static void -del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, +del_name(dns_rpz_zone_t *rpz, dns_rpz_type_t rpz_type, const dns_name_t *src_name) { char namebuf[DNS_NAME_FORMATSIZE]; dns_fixedname_t trig_namef; - dns_name_t *trig_name; - dns_rbtnode_t *nmnode; - dns_rpz_nm_data_t *nm_data, del_data; + dns_name_t *trig_name = NULL; + dns_rbtnode_t *nmnode = NULL; + dns_rpz_nm_data_t *nm_data = NULL; + dns_rpz_nm_data_t del_data; isc_result_t result; bool exists; @@ -2444,11 +2454,11 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, */ trig_name = dns_fixedname_initname(&trig_namef); - name2data(rpzs, rpz_num, rpz_type, src_name, trig_name, &del_data); + name2data(rpz, rpz_type, src_name, trig_name, &del_data); nmnode = NULL; - result = dns_rbt_findnode(rpzs->rbt, trig_name, NULL, &nmnode, NULL, 0, - NULL, NULL); + result = dns_rbt_findnode(rpz->rpzs->rbt, trig_name, NULL, &nmnode, + NULL, 0, NULL, NULL); if (result != ISC_R_SUCCESS) { /* * Do not worry about missing summary RBT nodes that probably @@ -2490,7 +2500,7 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, if (nm_data->set.qname == 0 && nm_data->set.ns == 0 && nm_data->wild.qname == 0 && nm_data->wild.ns == 0) { - result = dns_rbt_deletenode(rpzs->rbt, nmnode, false); + result = dns_rbt_deletenode(rpz->rpzs->rbt, nmnode, false); if (result != ISC_R_SUCCESS) { /* * bin/tests/system/rpz/tests.sh looks for @@ -2499,28 +2509,32 @@ del_name(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, dns_rpz_type_t rpz_type, dns_name_format(src_name, namebuf, sizeof(namebuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_RPZ, DNS_LOGMODULE_RBTDB, DNS_RPZ_ERROR_LEVEL, - "rpz del_name(%s) node delete failed: %s", + "rpz del_name(%s) node delete " + "failed: %s", namebuf, isc_result_totext(result)); } } if (exists) { - adj_trigger_cnt(rpzs, rpz_num, rpz_type, NULL, 0, false); + adj_trigger_cnt(rpz, rpz_type, NULL, 0, false); } } /* * Remove an IP address from the radix tree or a name from the summary database. */ -void -dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, - const dns_name_t *src_name) { - dns_rpz_zone_t *rpz; +static void +rpz_del(dns_rpz_zone_t *rpz, const dns_name_t *src_name) { dns_rpz_type_t rpz_type; + dns_rpz_zones_t *rpzs = NULL; + dns_rpz_num_t rpz_num; + + REQUIRE(rpz != NULL); + + rpzs = rpz->rpzs; + rpz_num = rpz->num; REQUIRE(rpzs != NULL && rpz_num < rpzs->p.num_zones); - rpz = rpzs->zones[rpz_num]; - REQUIRE(rpz != NULL); RWLOCK(&rpzs->search_lock, isc_rwlocktype_write); @@ -2529,12 +2543,12 @@ dns_rpz_delete(dns_rpz_zones_t *rpzs, dns_rpz_num_t rpz_num, switch (rpz_type) { case DNS_RPZ_TYPE_QNAME: case DNS_RPZ_TYPE_NSDNAME: - del_name(rpzs, rpz_num, rpz_type, src_name); + del_name(rpz, rpz_type, src_name); break; case DNS_RPZ_TYPE_CLIENT_IP: case DNS_RPZ_TYPE_IP: case DNS_RPZ_TYPE_NSIP: - del_cidr(rpzs, rpz_num, rpz_type, src_name); + del_cidr(rpz, rpz_type, src_name); break; case DNS_RPZ_TYPE_BAD: break; @@ -2557,7 +2571,7 @@ dns_rpz_find_ip(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, dns_name_t *ip_name, dns_rpz_prefix_t *prefixp) { dns_rpz_cidr_key_t tgt_ip; dns_rpz_addr_zbits_t tgt_set; - dns_rpz_cidr_node_t *found; + dns_rpz_cidr_node_t *found = NULL; isc_result_t result; dns_rpz_num_t rpz_num = 0; dns_rpz_have_t have; @@ -2675,8 +2689,8 @@ dns_rpz_zbits_t dns_rpz_find_name(dns_rpz_zones_t *rpzs, dns_rpz_type_t rpz_type, dns_rpz_zbits_t zbits, dns_name_t *trig_name) { char namebuf[DNS_NAME_FORMATSIZE]; - dns_rbtnode_t *nmnode; - const dns_rpz_nm_data_t *nm_data; + dns_rbtnode_t *nmnode = NULL; + const dns_rpz_nm_data_t *nm_data = NULL; dns_rpz_zbits_t found_zbits; dns_rbtnodechain_t chain; isc_result_t result; From e128b6a951ac1bce82aa686541d217cf51f9f6d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 7 Mar 2022 13:55:03 +0100 Subject: [PATCH 3/4] Run the RPZ update as offloaded work Previously, the RPZ updates ran quantized on the main nm_worker loops. As the quantum was set to 1024, this might lead to service interruptions when large RPZ update was processed. Change the RPZ update process to run as the offloaded work. The update and cleanup loops were refactored to do as little locking of the maintenance lock as possible for the shortest periods of time and the db iterator is being paused for every iteration, so we don't hold the rbtdb tree lock for prolonged periods of time. (cherry picked from commit f106d0ed2b8f3b3fa78b831879c1533a0d3ec171) --- lib/dns/include/dns/rpz.h | 34 +- lib/dns/rpz.c | 615 +++++++++++++++---------------------- lib/isc/include/isc/util.h | 10 + 3 files changed, 267 insertions(+), 392 deletions(-) diff --git a/lib/dns/include/dns/rpz.h b/lib/dns/include/dns/rpz.h index cb4be697e4..38dc9ffeab 100644 --- a/lib/dns/include/dns/rpz.h +++ b/lib/dns/include/dns/rpz.h @@ -144,25 +144,21 @@ struct dns_rpz_zone { dns_ttl_t max_policy_ttl; dns_rpz_policy_t policy; /* DNS_RPZ_POLICY_GIVEN or override */ - uint32_t min_update_interval; /* minimal interval between - * updates */ - isc_ht_t *nodes; /* entries in zone */ - dns_rpz_zones_t *rpzs; /* owner */ - isc_time_t lastupdated; /* last time the zone was processed - * */ - bool updatepending; /* there is an update - * pending/waiting */ - bool updaterunning; /* there is an update running */ - dns_db_t *db; /* zones database */ - dns_dbversion_t *dbversion; /* version we will be updating to */ - dns_db_t *updb; /* zones database we're working on */ - dns_dbversion_t *updbversion; /* version we're currently working - * on */ - dns_dbiterator_t *updbit; /* iterator to use when updating */ - isc_ht_t *newnodes; /* entries in zone being updated */ - bool db_registered; /* is the notify event - * registered? */ - bool addsoa; /* add soa to the additional section */ + uint32_t min_update_interval; /* minimal interval between + * updates */ + isc_ht_t *nodes; /* entries in zone */ + dns_rpz_zones_t *rpzs; /* owner */ + isc_time_t lastupdated; /* last time the zone was processed + * */ + bool updatepending; /* there is an update pending */ + bool updaterunning; /* there is an update running */ + isc_result_t updateresult; /* result from the offloaded work */ + dns_db_t *db; /* zones database */ + dns_dbversion_t *dbversion; /* version we will be updating to */ + dns_db_t *updb; /* zones database we're working on */ + dns_dbversion_t *updbversion; /* version we're currently working + * on */ + bool addsoa; /* add soa to the additional section */ isc_timer_t *updatetimer; isc_event_t updateevent; }; diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index 4bf4650c92..da8235e5da 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -88,11 +88,6 @@ #define DNS_RPZ_HTSIZE_MAX 24 #define DNS_RPZ_HTSIZE_DIV 3 -/* - * Maximum number of nodes to process per quantum - */ -#define DNS_RPZ_QUANTUM 1024 - static void update_from_db(dns_rpz_zone_t *rpz); @@ -172,6 +167,9 @@ struct dns_rpz_nm_data { dns_rpz_nm_zbits_t wild; }; +static isc_result_t +rpz_shuttingdown(dns_rpz_zone_t *rpz); + static isc_result_t rpz_add(dns_rpz_zone_t *rpz, const dns_name_t *src_name); static void @@ -1527,7 +1525,7 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) { /* * This will never be used, but costs us nothing and - * simplifies update_from_db + * simplifies update_from_db(). */ isc_ht_init(&rpz->nodes, rpzs->mctx, 1); @@ -1566,7 +1564,6 @@ isc_result_t dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { dns_rpz_zone_t *rpz = (dns_rpz_zone_t *)fn_arg; isc_time_t now; - uint64_t tdiff; isc_result_t result = ISC_R_SUCCESS; char dname[DNS_NAME_FORMATSIZE]; @@ -1591,15 +1588,18 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { dns_db_attach(db, &rpz->db); } + dns_name_format(&rpz->origin, dname, DNS_NAME_FORMATSIZE); + if (!rpz->updatepending && !rpz->updaterunning) { + uint64_t tdiff; + rpz->updatepending = true; + isc_time_now(&now); tdiff = isc_time_microdiff(&now, &rpz->lastupdated) / 1000000; if (tdiff < rpz->min_update_interval) { uint64_t defer = rpz->min_update_interval - tdiff; isc_interval_t interval; - dns_name_format(&rpz->origin, dname, - DNS_NAME_FORMATSIZE); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_INFO, "rpz: %s: new zone version came " @@ -1608,12 +1608,9 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { dname, defer); isc_interval_set(&interval, (unsigned int)defer, 0); dns_db_currentversion(rpz->db, &rpz->dbversion); - result = isc_timer_reset(rpz->updatetimer, - isc_timertype_once, NULL, - &interval, true); - if (result != ISC_R_SUCCESS) { - goto cleanup; - } + (void)isc_timer_reset(rpz->updatetimer, + isc_timertype_once, NULL, + &interval, true); } else { isc_event_t *event = NULL; @@ -1629,7 +1626,6 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { } } else { rpz->updatepending = true; - dns_name_format(&rpz->origin, dname, DNS_NAME_FORMATSIZE); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), "rpz: %s: update already queued or running", @@ -1639,8 +1635,6 @@ dns_rpz_dbupdate_callback(dns_db_t *db, void *fn_arg) { } dns_db_currentversion(rpz->db, &rpz->dbversion); } - -cleanup: UNLOCK(&rpz->rpzs->maint_lock); return (result); @@ -1672,36 +1666,75 @@ dns_rpz_update_taskaction(isc_task_t *task, isc_event_t *event) { UNLOCK(&rpz->rpzs->maint_lock); } +static void +update_rpz_done_cb(void *data, isc_result_t result) { + dns_rpz_zone_t *rpz = (dns_rpz_zone_t *)data; + char dname[DNS_NAME_FORMATSIZE]; + + if (result == ISC_R_SUCCESS && rpz->updateresult != ISC_R_SUCCESS) { + result = rpz->updateresult; + } + + LOCK(&rpz->rpzs->maint_lock); + rpz->updaterunning = false; + + dns_name_format(&rpz->origin, dname, DNS_NAME_FORMATSIZE); + + /* If there's no update pending, finish. */ + if (!rpz->updatepending) { + goto done; + } + + /* If there's an update pending, schedule it */ + if (rpz->min_update_interval > 0) { + uint64_t defer = rpz->min_update_interval; + isc_interval_t interval; + + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_MASTER, ISC_LOG_INFO, + "rpz: %s: new zone version came " + "too soon, deferring update for " + "%" PRIu64 " seconds", + dname, defer); + isc_interval_set(&interval, (unsigned int)defer, 0); + (void)isc_timer_reset(rpz->updatetimer, isc_timertype_once, + NULL, &interval, true); + } else { + isc_event_t *event = NULL; + INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); + ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, + NULL, DNS_EVENT_RPZUPDATED, + dns_rpz_update_taskaction, rpz, rpz, NULL, NULL); + event = &rpz->updateevent; + isc_task_send(rpz->rpzs->updater, &event); + } + +done: + dns_db_closeversion(rpz->updb, &rpz->updbversion, false); + dns_db_detach(&rpz->updb); + + UNLOCK(&rpz->rpzs->maint_lock); + + rpz_detach(&rpz); + + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, + ISC_LOG_INFO, "rpz: %s: reload done: %s", dname, + isc_result_totext(result)); +} + static isc_result_t -setup_update(dns_rpz_zone_t *rpz) { +update_nodes(dns_rpz_zone_t *rpz, isc_ht_t *newnodes) { isc_result_t result; + dns_dbiterator_t *updbit = NULL; + dns_name_t *name = NULL; + dns_fixedname_t fixname; char domain[DNS_NAME_FORMATSIZE]; - unsigned int nodecount; - uint32_t hashsize; dns_name_format(&rpz->origin, domain, DNS_NAME_FORMATSIZE); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, - ISC_LOG_INFO, "rpz: %s: reload start", domain); - nodecount = dns_db_nodecount(rpz->updb, dns_dbtree_main); - hashsize = 1; - while (nodecount != 0 && - hashsize <= (DNS_RPZ_HTSIZE_MAX + DNS_RPZ_HTSIZE_DIV)) { - hashsize++; - nodecount >>= 1; - } + name = dns_fixedname_initname(&fixname); - if (hashsize > DNS_RPZ_HTSIZE_DIV) { - hashsize -= DNS_RPZ_HTSIZE_DIV; - } - - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, - ISC_LOG_DEBUG(1), "rpz: %s: using hashtable size %d", - domain, hashsize); - - isc_ht_init(&rpz->newnodes, rpz->rpzs->mctx, hashsize); - - result = dns_db_createiterator(rpz->updb, DNS_DB_NONSEC3, &rpz->updbit); + result = dns_db_createiterator(rpz->updb, DNS_DB_NONSEC3, &updbit); if (result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, @@ -1710,8 +1743,8 @@ setup_update(dns_rpz_zone_t *rpz) { goto cleanup; } - result = dns_dbiterator_first(rpz->updbit); - if (result != ISC_R_SUCCESS) { + result = dns_dbiterator_first(updbit); + if (result != ISC_R_SUCCESS && result != ISC_R_NOMORE) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "rpz: %s: failed to get db iterator - %s", domain, @@ -1719,220 +1752,30 @@ setup_update(dns_rpz_zone_t *rpz) { goto cleanup; } - result = dns_dbiterator_pause(rpz->updbit); - if (result != ISC_R_SUCCESS) { - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, - "rpz: %s: failed to pause db iterator - %s", - domain, isc_result_totext(result)); - goto cleanup; - } - -cleanup: - if (result != ISC_R_SUCCESS) { - if (rpz->updbit != NULL) { - dns_dbiterator_destroy(&rpz->updbit); - } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); - } - dns_db_closeversion(rpz->updb, &rpz->updbversion, false); - } - - return (result); -} - -static void -finish_update(dns_rpz_zone_t *rpz) { - LOCK(&rpz->rpzs->maint_lock); - rpz->updaterunning = false; - - /* - * If there's an update pending, schedule it. - */ - if (rpz->updatepending) { - if (rpz->min_update_interval > 0) { - uint64_t defer = rpz->min_update_interval; - char dname[DNS_NAME_FORMATSIZE]; - isc_interval_t interval; - - dns_name_format(&rpz->origin, dname, - DNS_NAME_FORMATSIZE); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, ISC_LOG_INFO, - "rpz: %s: new zone version came " - "too soon, deferring update for " - "%" PRIu64 " seconds", - dname, defer); - isc_interval_set(&interval, (unsigned int)defer, 0); - isc_timer_reset(rpz->updatetimer, isc_timertype_once, - NULL, &interval, true); - } else { - isc_event_t *event = NULL; - INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); - ISC_EVENT_INIT(&rpz->updateevent, - sizeof(rpz->updateevent), 0, NULL, - DNS_EVENT_RPZUPDATED, - dns_rpz_update_taskaction, rpz, rpz, - NULL, NULL); - event = &rpz->updateevent; - isc_task_send(rpz->rpzs->updater, &event); - } - } - UNLOCK(&rpz->rpzs->maint_lock); -} - -static void -cleanup_quantum(isc_task_t *task, isc_event_t *event) { - isc_result_t result = ISC_R_SUCCESS; - char domain[DNS_NAME_FORMATSIZE]; - dns_rpz_zone_t *rpz = NULL; - isc_ht_iter_t *iter = NULL; - dns_fixedname_t fname; - dns_name_t *name = NULL; - int count = 0; - - UNUSED(task); - - REQUIRE(event != NULL); - REQUIRE(event->ev_sender != NULL); - - rpz = (dns_rpz_zone_t *)event->ev_sender; - iter = (isc_ht_iter_t *)event->ev_arg; - isc_event_free(&event); - - if (iter == NULL) { - /* - * Iterate over old ht with existing nodes deleted to - * delete deleted nodes from RPZ - */ - isc_ht_iter_create(rpz->nodes, &iter); - } - - name = dns_fixedname_initname(&fname); - - LOCK(&rpz->rpzs->maint_lock); - - /* Check that we aren't shutting down. */ - if (rpz->rpzs->zones[rpz->num] == NULL) { - UNLOCK(&rpz->rpzs->maint_lock); - goto cleanup; - } - - for (result = isc_ht_iter_first(iter); - result == ISC_R_SUCCESS && count++ < DNS_RPZ_QUANTUM; - result = isc_ht_iter_delcurrent_next(iter)) - { - isc_region_t region; - unsigned char *key = NULL; - size_t keysize; - - isc_ht_iter_currentkey(iter, &key, &keysize); - region.base = key; - region.length = (unsigned int)keysize; - dns_name_fromregion(name, ®ion); - rpz_del(rpz, name); - } - - if (result == ISC_R_SUCCESS) { - isc_event_t *nevent = NULL; - - /* - * We finished a quantum; trigger the next one and return. - */ - - INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); - ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, - NULL, DNS_EVENT_RPZUPDATED, cleanup_quantum, - iter, rpz, NULL, NULL); - nevent = &rpz->updateevent; - isc_task_send(rpz->rpzs->updater, &nevent); - UNLOCK(&rpz->rpzs->maint_lock); - return; - } else if (result == ISC_R_NOMORE) { - isc_ht_t *tmpht = NULL; - - /* - * Done with cleanup of deleted nodes; finalize - * the update. - */ - tmpht = rpz->nodes; - rpz->nodes = rpz->newnodes; - rpz->newnodes = tmpht; - - UNLOCK(&rpz->rpzs->maint_lock); - finish_update(rpz); - dns_name_format(&rpz->origin, domain, DNS_NAME_FORMATSIZE); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, ISC_LOG_INFO, - "rpz: %s: reload done", domain); - } else { - UNLOCK(&rpz->rpzs->maint_lock); - } - - /* - * If we're here, we're finished or something went wrong. - */ -cleanup: - if (iter != NULL) { - isc_ht_iter_destroy(&iter); - } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); - } - dns_db_closeversion(rpz->updb, &rpz->updbversion, false); - dns_db_detach(&rpz->updb); - rpz_detach(&rpz); -} - -static void -update_quantum(isc_task_t *task, isc_event_t *event) { - isc_result_t result = ISC_R_SUCCESS; - dns_dbnode_t *node = NULL; - dns_rpz_zone_t *rpz = NULL; - char domain[DNS_NAME_FORMATSIZE]; - dns_fixedname_t fixname; - dns_name_t *name = NULL; - isc_event_t *nevent = NULL; - int count = 0; - - UNUSED(task); - - REQUIRE(event != NULL); - REQUIRE(event->ev_arg != NULL); - - rpz = (dns_rpz_zone_t *)event->ev_arg; - isc_event_free(&event); - - REQUIRE(rpz->updbit != NULL); - REQUIRE(rpz->newnodes != NULL); - - name = dns_fixedname_initname(&fixname); - - dns_name_format(&rpz->origin, domain, DNS_NAME_FORMATSIZE); - - LOCK(&rpz->rpzs->maint_lock); - - /* Check that we aren't shutting down. */ - if (rpz->rpzs->zones[rpz->num] == NULL) { - UNLOCK(&rpz->rpzs->maint_lock); - goto cleanup; - } - - while (result == ISC_R_SUCCESS && count++ < DNS_RPZ_QUANTUM) { + while (result == ISC_R_SUCCESS) { char namebuf[DNS_NAME_FORMATSIZE]; dns_rdatasetiter_t *rdsiter = NULL; + dns_dbnode_t *node = NULL; - result = dns_dbiterator_current(rpz->updbit, &node, name); + result = rpz_shuttingdown(rpz); + if (result != ISC_R_SUCCESS) { + dns_db_detachnode(rpz->updb, &node); + goto cleanup; + } + + result = dns_dbiterator_current(updbit, &node, name); if (result != ISC_R_SUCCESS) { isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, "rpz: %s: failed to get dbiterator - %s", domain, isc_result_totext(result)); dns_db_detachnode(rpz->updb, &node); - break; + goto cleanup; } + result = dns_dbiterator_pause(updbit); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + result = dns_db_allrdatasets(rpz->updb, node, rpz->updbversion, 0, &rdsiter); if (result != ISC_R_SUCCESS) { @@ -1942,12 +1785,15 @@ update_quantum(isc_task_t *task, isc_event_t *event) { "rrdatasets - %s", domain, isc_result_totext(result)); dns_db_detachnode(rpz->updb, &node); - break; + goto cleanup; } result = dns_rdatasetiter_first(rdsiter); + dns_rdatasetiter_destroy(&rdsiter); - if (result != ISC_R_SUCCESS) { /* empty non-terminal */ + dns_db_detachnode(rpz->updb, &node); + + if (result != ISC_R_SUCCESS) { /* skip empty non-terminal */ if (result != ISC_R_NOMORE) { isc_log_write( dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -1956,14 +1802,13 @@ update_quantum(isc_task_t *task, isc_event_t *event) { "rdatasetiter", domain, isc_result_totext(result)); } - dns_db_detachnode(rpz->updb, &node); - result = dns_dbiterator_next(rpz->updbit); - continue; + goto next; } dns_name_downcase(name, name, NULL); - result = isc_ht_add(rpz->newnodes, name->ndata, name->length, - rpz); + + /* Add entry to the new nodes table */ + result = isc_ht_add(newnodes, name->ndata, name->length, rpz); if (result != ISC_R_SUCCESS) { dns_name_format(name, namebuf, sizeof(namebuf)); isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, @@ -1971,131 +1816,170 @@ update_quantum(isc_task_t *task, isc_event_t *event) { "rpz: %s, adding node %s to HT error %s", domain, namebuf, isc_result_totext(result)); - dns_db_detachnode(rpz->updb, &node); - result = dns_dbiterator_next(rpz->updbit); - continue; + goto next; } + /* Does the entry exist in the old nodes table? */ result = isc_ht_find(rpz->nodes, name->ndata, name->length, NULL); - if (result == ISC_R_SUCCESS) { + if (result == ISC_R_SUCCESS) { /* found */ isc_ht_delete(rpz->nodes, name->ndata, name->length); - } else { /* not found */ - result = rpz_add(rpz, name); - if (result != ISC_R_SUCCESS) { - dns_name_format(name, namebuf, sizeof(namebuf)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, - ISC_LOG_ERROR, - "rpz: %s: adding node %s " - "to RPZ error %s", - domain, namebuf, - isc_result_totext(result)); - } else { - dns_name_format(name, namebuf, sizeof(namebuf)); - isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, - DNS_LOGMODULE_MASTER, - ISC_LOG_DEBUG(3), - "rpz: %s: adding node %s", domain, - namebuf); - } + goto next; } - dns_db_detachnode(rpz->updb, &node); - result = dns_dbiterator_next(rpz->updbit); - } - - if (result == ISC_R_SUCCESS) { /* - * Pause the iterator so that the DB is not locked. + * Only the single rpz updates are serialized, so we need to + * lock here because we can be processing more updates to + * different rpz zones at the same time */ - dns_dbiterator_pause(rpz->updbit); - - /* - * We finished a quantum; trigger the next one and return. - */ - INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); - ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, - NULL, DNS_EVENT_RPZUPDATED, update_quantum, rpz, - rpz, NULL, NULL); - nevent = &rpz->updateevent; - isc_task_send(rpz->rpzs->updater, &nevent); + LOCK(&rpz->rpzs->maint_lock); + result = rpz_add(rpz, name); UNLOCK(&rpz->rpzs->maint_lock); - return; - } else if (result == ISC_R_NOMORE) { - /* - * Done with the new database; now we just need to - * clean up the old. - */ - dns_dbiterator_destroy(&rpz->updbit); - INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); - ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, - NULL, DNS_EVENT_RPZUPDATED, cleanup_quantum, - NULL, rpz, NULL, NULL); - nevent = &rpz->updateevent; - isc_task_send(rpz->rpzs->updater, &nevent); - UNLOCK(&rpz->rpzs->maint_lock); - return; + if (result != ISC_R_SUCCESS) { + dns_name_format(name, namebuf, sizeof(namebuf)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_MASTER, ISC_LOG_ERROR, + "rpz: %s: adding node %s " + "to RPZ error %s", + domain, namebuf, + isc_result_totext(result)); + } else if (isc_log_wouldlog(dns_lctx, ISC_LOG_DEBUG(3))) { + dns_name_format(name, namebuf, sizeof(namebuf)); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, + DNS_LOGMODULE_MASTER, ISC_LOG_DEBUG(3), + "rpz: %s: adding node %s", domain, + namebuf); + } + + next: + result = dns_dbiterator_next(updbit); + } + INSIST(result != ISC_R_SUCCESS); + if (result == ISC_R_NOMORE) { + result = ISC_R_SUCCESS; } - - /* - * If we're here, something went wrong, so clean up. - */ - UNLOCK(&rpz->rpzs->maint_lock); cleanup: - if (rpz->updbit != NULL) { - dns_dbiterator_destroy(&rpz->updbit); + dns_dbiterator_destroy(&updbit); + + return (result); +} + +static isc_result_t +cleanup_nodes(dns_rpz_zone_t *rpz) { + isc_result_t result; + isc_ht_iter_t *iter = NULL; + dns_name_t *name = NULL; + dns_fixedname_t fixname; + + name = dns_fixedname_initname(&fixname); + + isc_ht_iter_create(rpz->nodes, &iter); + + for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS; + result = isc_ht_iter_delcurrent_next(iter)) + { + isc_region_t region; + unsigned char *key = NULL; + size_t keysize; + + result = rpz_shuttingdown(rpz); + if (result != ISC_R_SUCCESS) { + break; + } + + isc_ht_iter_currentkey(iter, &key, &keysize); + region.base = key; + region.length = (unsigned int)keysize; + dns_name_fromregion(name, ®ion); + + LOCK(&rpz->rpzs->maint_lock); + rpz_del(rpz, name); + UNLOCK(&rpz->rpzs->maint_lock); } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); + INSIST(result != ISC_R_SUCCESS); + if (result == ISC_R_NOMORE) { + result = ISC_R_SUCCESS; } - dns_db_closeversion(rpz->updb, &rpz->updbversion, false); - dns_db_detach(&rpz->updb); - rpz_detach(&rpz); + + isc_ht_iter_destroy(&iter); + + return (result); +} + +static isc_result_t +rpz_shuttingdown(dns_rpz_zone_t *rpz) { + bool shuttingdown = false; + + LOCK(&rpz->rpzs->maint_lock); + /* Check that we aren't shutting down. */ + shuttingdown = (rpz->rpzs->zones[rpz->num] == NULL); + UNLOCK(&rpz->rpzs->maint_lock); + + if (shuttingdown) { + return (ISC_R_SHUTTINGDOWN); + } + + return (ISC_R_SUCCESS); } static void -update_from_db(dns_rpz_zone_t *rpz) { - dns_rpz_zone_t *rpz_zone = NULL; +update_rpz_cb(void *data) { + dns_rpz_zone_t *rpz = (dns_rpz_zone_t *)data; + isc_result_t result = ISC_R_SUCCESS; + isc_ht_t *newnodes = NULL; - REQUIRE(rpz != NULL); - REQUIRE(DNS_DB_VALID(rpz->db)); - REQUIRE(rpz->updb == NULL); - REQUIRE(rpz->updbversion == NULL); - REQUIRE(rpz->updbit == NULL); - REQUIRE(rpz->newnodes == NULL); + REQUIRE(rpz->nodes != NULL); - rpz_attach(rpz, &(dns_rpz_zone_t *){ NULL }); - - dns_db_attach(rpz->db, &rpz->updb); - rpz->updbversion = rpz->dbversion; - rpz->dbversion = NULL; - - result = setup_update(rpz); + result = rpz_shuttingdown(rpz); if (result != ISC_R_SUCCESS) { goto cleanup; } - event = &rpz->updateevent; - INSIST(!ISC_LINK_LINKED(&rpz->updateevent, ev_link)); - ISC_EVENT_INIT(&rpz->updateevent, sizeof(rpz->updateevent), 0, NULL, - DNS_EVENT_RPZUPDATED, update_quantum, rpz, rpz, NULL, - NULL); - isc_task_send(rpz->rpzs->updater, &event); - return; + isc_ht_init(&newnodes, rpz->rpzs->mctx, 1); + + result = update_nodes(rpz, newnodes); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + result = cleanup_nodes(rpz); + if (result != ISC_R_SUCCESS) { + goto cleanup; + } + + /* Finalize the update */ + ISC_SWAP(rpz->nodes, newnodes); cleanup: - if (rpz->updbit != NULL) { - dns_dbiterator_destroy(&rpz->updbit); - } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); - } - dns_db_closeversion(rpz->updb, &rpz->updbversion, false); - dns_db_detach(&rpz->updb); - rpz_detach(&rpz); + isc_ht_destroy(&newnodes); + + rpz->updateresult = result; +} + +static void +update_from_db(dns_rpz_zone_t *rpz) { + char domain[DNS_NAME_FORMATSIZE]; + dns_rpz_zone_t *rpz_zone = NULL; + + REQUIRE(isc_nm_tid() >= 0); + REQUIRE(rpz != NULL); + REQUIRE(DNS_DB_VALID(rpz->db)); + REQUIRE(rpz->updb == NULL); + REQUIRE(rpz->updbversion == NULL); + + rpz_attach(rpz, &rpz_zone); + dns_db_attach(rpz->db, &rpz->updb); + rpz->updbversion = rpz->dbversion; + rpz->dbversion = NULL; + + dns_name_format(&rpz->origin, domain, DNS_NAME_FORMATSIZE); + isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER, + ISC_LOG_INFO, "rpz: %s: reload start", domain); + + isc_nm_work_offload(isc_task_getnetmgr(rpz->rpzs->updater), + update_rpz_cb, update_rpz_done_cb, rpz_zone); } /* @@ -2182,22 +2066,8 @@ rpz_destroy(dns_rpz_zone_t *rpz) { dns_rpz_dbupdate_callback, rpz); dns_db_detach(&rpz->db); } - if (rpz->updaterunning) { - isc_task_purgeevent(rpzs->updater, &rpz->updateevent); - if (rpz->updbit != NULL) { - dns_dbiterator_destroy(&rpz->updbit); - } - if (rpz->newnodes != NULL) { - isc_ht_destroy(&rpz->newnodes); - } - if (rpz->updb != NULL) { - if (rpz->updbversion != NULL) { - dns_db_closeversion(rpz->updb, - &rpz->updbversion, false); - } - dns_db_detach(&rpz->updb); - } - } + + INSIST(!rpz->updaterunning); isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, NULL, true); @@ -2244,20 +2114,19 @@ dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) { *rpzsp = NULL; if (isc_refcount_decrement(&rpzs->refs) == 1) { - LOCK(&rpzs->maint_lock); /* - * Forget the last of view's rpz machinery after + * Forget the last of the view's rpz machinery after * the last reference. */ + LOCK(&rpzs->maint_lock); for (dns_rpz_num_t rpz_num = 0; rpz_num < DNS_RPZ_MAX_ZONES; ++rpz_num) { - dns_rpz_zone_t *rpz = rpzs->zones[rpz_num]; - rpzs->zones[rpz_num] = NULL; - if (rpz != NULL) { - rpz_detach(&rpz); + if (rpzs->zones[rpz_num] != NULL) { + rpz_detach(&rpzs->zones[rpz_num]); } } UNLOCK(&rpzs->maint_lock); + rpz_detach_rpzs(&rpzs); } } diff --git a/lib/isc/include/isc/util.h b/lib/isc/include/isc/util.h index 63d3af340c..4494bbbdeb 100644 --- a/lib/isc/include/isc/util.h +++ b/lib/isc/include/isc/util.h @@ -344,3 +344,13 @@ mock_assert(const int result, const char *const expression, * Misc */ #include + +/*% + * Swap + */ +#define ISC_SWAP(a, b) \ + { \ + typeof(a) __tmp_swap = a; \ + a = b; \ + b = __tmp_swap; \ + } From f3ae14d8c3412401ba4c7160cfd5408da3f6b1a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 8 Mar 2022 18:36:08 +0100 Subject: [PATCH 4/4] Add CHANGES and release note for [GL #3190] (cherry picked from commit 23a4559b3496e392e1106de3fe263baa5b01aa74) --- CHANGES | 3 +++ doc/notes/notes-current.rst | 5 +++++ 2 files changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index cab4f1807c..96c4663d9a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5850. [func] Run the RPZ update process on the offload threads. + [GL #3190] + 5848. [bug] dig could hang in some cases involving multiple servers in a lookup, when a request fails and the next one refuses to start for some reason, for example if it was diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index d7ec2821a3..4e6d06c90d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -27,6 +27,11 @@ New Features - None. +- Run RPZ updates on the specialized "offload" threads to reduce the amount + of time they block query processing on the main networking threads. This + should increase the responsiveness of ``named`` when RPZ updates are being + applied after an RPZ zone has been successfully transfered. :gl:`#3190` + Removed Features ~~~~~~~~~~~~~~~~