diff --git a/CHANGES b/CHANGES index acbbaf255a..b3ea4ca14e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6039. [bug] Removing a catalog zone from catalog-zones without + also removing the referenced zone could leave a + dangling pointer. [GL #3683] + 6038. [bug] In some serve stale scenarios, like when following an expired CNAME record, named could return SERVFAIL if the previous request wasn't successful. Consider non-stale diff --git a/bin/tests/system/catz/tests.sh b/bin/tests/system/catz/tests.sh index 697ae9badf..ee95d2e124 100644 --- a/bin/tests/system/catz/tests.sh +++ b/bin/tests/system/catz/tests.sh @@ -2515,6 +2515,45 @@ rndccmd 10.53.0.2 reconfig || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) +######################################################################### + +nextpart ns2/named.run >/dev/null + +n=$((n+1)) +echo_i "Adding a dom19.example. to primary via RNDC ($n)" +ret=0 +# enough initial content for IXFR response when TXT record is added below +echo "@ 3600 IN SOA . . 1 3600 3600 3600 3600" > ns1/dom19.example.db +echo "@ 3600 IN NS invalid." >> ns1/dom19.example.db +echo "foo 3600 IN TXT some content here" >> ns1/dom19.example.db +echo "bar 3600 IN TXT some content here" >> ns1/dom19.example.db +echo "xxx 3600 IN TXT some content here" >> ns1/dom19.example.db +echo "yyy 3600 IN TXT some content here" >> ns1/dom19.example.db +rndccmd 10.53.0.1 addzone dom19.example. in default '{ type primary; file "dom19.example.db"; allow-transfer { key tsig_key; }; allow-update { any; }; notify explicit; also-notify { 10.53.0.2; }; };' || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "add an entry to the restored catalog zone ($n)" +ret=0 +$NSUPDATE -d <> nsupdate.out.test$n 2>&1 || ret=1 + server 10.53.0.1 ${PORT} + update add 09da0a318e5333a9a7f6c14c385d69f6933e8b72.zones.catalog1.example. 3600 IN PTR dom19.example. + update add label1.primaries.ext.09da0a318e5333a9a7f6c14c385d69f6933e8b72.zones.catalog1.example. 3600 IN A 10.53.0.1 + update add label1.primaries.ext.09da0a318e5333a9a7f6c14c385d69f6933e8b72.zones.catalog1.example. 3600 IN TXT "tsig_key" + send +END +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + +n=$((n+1)) +echo_i "waiting for secondary to sync up ($n)" +ret=0 +wait_for_message ns2/named.run "catz: adding zone 'dom19.example' from catalog 'catalog1.example'" && +wait_for_message ns2/named.run "transfer of 'dom19.example/IN/default' from 10.53.0.1#${PORT}: Transfer status: success" || ret=1 +if [ $ret -ne 0 ]; then echo_i "failed"; fi +status=$((status+ret)) + ########################################################################## n=$((n+1)) echo_i "Adding a domain tls1.example. to primary via RNDC ($n)" diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 3f03dd0bd1..49e2931b8d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -71,6 +71,10 @@ Bug Fixes names in zone transfers that should have been compressed, so zone transfers were larger than before. :gl:`#3706` +- When a catalog zone is removed from the configuration, in some + cases a dangling pointer could cause a :iscman:`named` process + crash. This has been fixed. :gl:`#3683` + Known Issues ~~~~~~~~~~~~ diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 2b8b64b5e2..ec048dfd8f 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -791,7 +791,7 @@ cleanup_ht: void dns_catz_catzs_set_view(dns_catz_zones_t *catzs, dns_view_t *view) { REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); - REQUIRE(view != NULL); + REQUIRE(DNS_VIEW_VALID(view)); /* Either it's a new one or it's being reconfigured. */ REQUIRE(catzs->view == NULL || !strcmp(catzs->view->name, view->name)); @@ -982,9 +982,8 @@ dns__catz_zone_destroy(dns_catz_zone_t *zone) { } if (zone->db_registered) { - INSIST(dns_db_updatenotify_unregister( - zone->db, dns_catz_dbupdate_callback, - zone->catzs) == ISC_R_SUCCESS); + dns_db_updatenotify_unregister( + zone->db, dns_catz_dbupdate_callback, zone->catzs); } if (zone->dbversion) { dns_db_closeversion(zone->db, &zone->dbversion, false); @@ -1007,7 +1006,7 @@ void dns_catz_catzs_detach(dns_catz_zones_t **catzsp) { dns_catz_zones_t *catzs; - REQUIRE(catzsp != NULL && *catzsp != NULL); + REQUIRE(catzsp != NULL && DNS_CATZ_ZONES_VALID(*catzsp)); catzs = *catzsp; *catzsp = NULL; @@ -1834,7 +1833,7 @@ dns_catz_generate_masterfilename(dns_catz_zone_t *zone, dns_catz_entry_t *entry, bool special = false; REQUIRE(DNS_CATZ_ZONE_VALID(zone)); - REQUIRE(entry != NULL); + REQUIRE(DNS_CATZ_ENTRY_VALID(entry)); REQUIRE(buffer != NULL && *buffer != NULL); isc_buffer_allocate(zone->catzs->mctx, &tbuf, @@ -1932,7 +1931,7 @@ dns_catz_generate_zonecfg(dns_catz_zone_t *zone, dns_catz_entry_t *entry, char zname[DNS_NAME_FORMATSIZE]; REQUIRE(DNS_CATZ_ZONE_VALID(zone)); - REQUIRE(entry != NULL); + REQUIRE(DNS_CATZ_ENTRY_VALID(entry)); REQUIRE(buf != NULL && *buf == NULL); /* @@ -2067,7 +2066,7 @@ dns_catz_dbupdate_callback(dns_db_t *db, void *fn_arg) { isc_region_t r; REQUIRE(DNS_DB_VALID(db)); - REQUIRE(fn_arg != NULL); + REQUIRE(DNS_CATZ_ZONES_VALID(fn_arg)); catzs = (dns_catz_zones_t *)fn_arg; dns_name_toregion(&db->origin, &r); diff --git a/lib/dns/db.c b/lib/dns/db.c index 5ea9532816..9620efb165 100644 --- a/lib/dns/db.c +++ b/lib/dns/db.c @@ -995,7 +995,7 @@ dns_db_rpz_ready(dns_db_t *db) { return ((db->methods->rpz_ready)(db)); } -/** +/* * Attach a notify-on-update function the database */ isc_result_t @@ -1006,6 +1006,16 @@ dns_db_updatenotify_register(dns_db_t *db, dns_dbupdate_callback_t fn, REQUIRE(db != NULL); REQUIRE(fn != NULL); + for (listener = ISC_LIST_HEAD(db->update_listeners); listener != NULL; + listener = ISC_LIST_NEXT(listener, link)) + { + if ((listener->onupdate == fn) && + (listener->onupdate_arg == fn_arg)) + { + return (ISC_R_SUCCESS); + } + } + listener = isc_mem_get(db->mctx, sizeof(dns_dbonupdatelistener_t)); listener->onupdate = fn; diff --git a/lib/dns/include/dns/db.h b/lib/dns/include/dns/db.h index 1831473e51..f1ca17973b 100644 --- a/lib/dns/include/dns/db.h +++ b/lib/dns/include/dns/db.h @@ -1634,11 +1634,11 @@ dns_db_updatenotify_register(dns_db_t *db, dns_dbupdate_callback_t fn, void *fn_arg); /*%< * Register a notify-on-update callback function to a database. + * Duplicate callbacks are suppressed. * * Requires: * * \li 'db' is a valid database - * \li 'db' does not have an update callback registered * \li 'fn' is not NULL * */ diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 8cf4cc3aaa..a029c6e43f 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -2621,7 +2621,8 @@ dns_zone_catz_enable(dns_zone_t *zone, dns_catz_zones_t *catzs); void dns_zone_catz_disable(dns_zone_t *zone); /*%< - * Disable zone as catalog zone, if it is one. + * Disable zone as catalog zone, if it is one. Also disables any + * registered callbacks for the catalog zone. * * Requires: * diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index cf8170e874..15732ca87b 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -1112,7 +1112,6 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log) { char buf[DNS_NAME_FORMATSIZE]; dns_rbt_t **treep; isc_time_t start; - dns_dbonupdatelistener_t *listener, *listener_next; if (IS_CACHE(rbtdb) && rbtdb->common.rdclass == dns_rdataclass_in) { overmem((dns_db_t *)rbtdb, (bool)-1); @@ -1253,14 +1252,7 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log) { rbtdb->common.impmagic = 0; isc_mem_detach(&rbtdb->hmctx); - for (listener = ISC_LIST_HEAD(rbtdb->common.update_listeners); - listener != NULL; listener = listener_next) - { - listener_next = ISC_LIST_NEXT(listener, link); - ISC_LIST_UNLINK(rbtdb->common.update_listeners, listener, link); - isc_mem_put(rbtdb->common.mctx, listener, - sizeof(dns_dbonupdatelistener_t)); - } + INSIST(ISC_LIST_EMPTY(rbtdb->common.update_listeners)); isc_mem_putanddetach(&rbtdb->common.mctx, rbtdb, sizeof(*rbtdb)); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index fb718151cc..bab33f390d 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1981,6 +1981,31 @@ dns_zone_rpz_disable_db(dns_zone_t *zone, dns_db_t *db) { zone->rpzs->zones[zone->rpz_num]); } +/* + * If a zone is a catalog zone, attach it to update notification in database. + */ +void +dns_zone_catz_enable_db(dns_zone_t *zone, dns_db_t *db) { + REQUIRE(DNS_ZONE_VALID(zone)); + REQUIRE(db != NULL); + + if (zone->catzs != NULL) { + dns_db_updatenotify_register(db, dns_catz_dbupdate_callback, + zone->catzs); + } +} + +static void +dns_zone_catz_disable_db(dns_zone_t *zone, dns_db_t *db) { + REQUIRE(DNS_ZONE_VALID(zone)); + REQUIRE(db != NULL); + + if (zone->catzs != NULL) { + dns_db_updatenotify_unregister(db, dns_catz_dbupdate_callback, + zone->catzs); + } +} + static void zone_catz_enable(dns_zone_t *zone, dns_catz_zones_t *catzs) { REQUIRE(DNS_ZONE_VALID(zone)); @@ -2007,6 +2032,9 @@ zone_catz_disable(dns_zone_t *zone) { REQUIRE(DNS_ZONE_VALID(zone)); if (zone->catzs != NULL) { + if (zone->db != NULL) { + dns_zone_catz_disable_db(zone, zone->db); + } dns_catz_catzs_detach(&zone->catzs); } } @@ -2027,31 +2055,6 @@ dns_zone_catz_is_enabled(dns_zone_t *zone) { return (zone->catzs != NULL); } -/* - * If a zone is a catalog zone, attach it to update notification in database. - */ -void -dns_zone_catz_enable_db(dns_zone_t *zone, dns_db_t *db) { - REQUIRE(DNS_ZONE_VALID(zone)); - REQUIRE(db != NULL); - - if (zone->catzs != NULL) { - dns_db_updatenotify_register(db, dns_catz_dbupdate_callback, - zone->catzs); - } -} - -static void -dns_zone_catz_disable_db(dns_zone_t *zone, dns_db_t *db) { - REQUIRE(DNS_ZONE_VALID(zone)); - REQUIRE(db != NULL); - - if (zone->catzs != NULL) { - dns_db_updatenotify_unregister(db, dns_catz_dbupdate_callback, - zone->catzs); - } -} - /* * Set catalog zone ownership of the zone */ @@ -5456,6 +5459,11 @@ cleanup: isc_result_totext(result)); } + if (result != ISC_R_SUCCESS) { + dns_zone_rpz_disable_db(zone, db); + dns_zone_catz_disable_db(zone, db); + } + for (inc = ISC_LIST_HEAD(zone->newincludes); inc != NULL; inc = ISC_LIST_HEAD(zone->newincludes)) { @@ -17706,6 +17714,8 @@ static void zone_detachdb(dns_zone_t *zone) { REQUIRE(zone->db != NULL); + dns_zone_rpz_disable_db(zone, zone->db); + dns_zone_catz_disable_db(zone, zone->db); dns_db_detach(&zone->db); }