Merge branch '3683-use-after-free-in-catalog-zone-processing' into 'main'

Resolve "use after free in catalog zone processing"

Closes #3683

See merge request isc-projects/bind9!7137
This commit is contained in:
Mark Andrews 2022-12-06 22:29:46 +00:00
commit 8f6a0c85ea
9 changed files with 104 additions and 45 deletions

View file

@ -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

View file

@ -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 <<END >> 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)"

View file

@ -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
~~~~~~~~~~~~

View file

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

View file

@ -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;

View file

@ -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
*
*/

View file

@ -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:
*

View file

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

View file

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