From 14926c3403e30bb7274e1dde7d3fa01dd1c53c0f Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 25 Feb 2015 16:12:34 -0800 Subject: [PATCH] [v9_9] fix LOADPENDING issues 4063. [bug] Asynchronous zone loads were not handled correctly when the zone load was already in progress; this could trigger a crash in zt.c. [RT #37573] (cherry picked from commit 7acc2f21563b79229d592f09dde17e60d64afc8f) (cherry picked from commit 62fd632bcbfeebf0e2c6231fffe7b3b7f94f80db) --- CHANGES | 5 +++++ doc/arm/notes.xml | 9 +++++++- lib/dns/include/dns/zone.h | 9 ++++++++ lib/dns/zone.c | 43 ++++++++++++++++++++++++++------------ 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/CHANGES b/CHANGES index e4d5c2dcfb..6b6d6f790a 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,8 @@ +4063. [bug] Asynchronous zone loads were not handled + correctly when the zone load was already in + progress; this could trigger a crash in zt.c. + [RT #37573] + 4062. [bug] Fix an out-of-bounds read in RPZ code. If the read succeeded, it doesn't result in a bug during operation. If the read failed, named diff --git a/doc/arm/notes.xml b/doc/arm/notes.xml index 6960bda51b..4268cc50f5 100644 --- a/doc/arm/notes.xml +++ b/doc/arm/notes.xml @@ -266,7 +266,14 @@ Fixed some bugs in RFC 5011 trust anchor management, including a memory leak and a possible loss of state - information.[RT #38458] + information. [RT #38458] + + + + + Asynchronous zone loads were not handled correctly when the + zone load was already in progress; this could trigger a crash + in zt.c. [RT #37573] diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 9efa1e7297..53b8302765 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -331,6 +331,15 @@ dns_zone_asyncload(dns_zone_t *zone, dns_zt_zoneloaded_t done, void *arg); * its first argument and 'zone' as its second. (Normally, 'arg' is * expected to point to the zone table but is left undefined for testing * purposes.) + * + * Require: + *\li 'zone' to be a valid zone. + * + * Returns: + *\li #ISC_R_ALREADYRUNNING + *\li #ISC_R_SUCCESS + *\li #ISC_R_FAILURE + *\li #ISC_R_NOMEMORY */ isc_boolean_t diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 11ad1ce5ad..fef4b28998 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -1590,7 +1590,7 @@ dns_zone_rpz_enable_db(dns_zone_t *zone, dns_db_t *db) { } static isc_result_t -zone_load(dns_zone_t *zone, unsigned int flags) { +zone_load(dns_zone_t *zone, unsigned int flags, isc_boolean_t locked) { isc_result_t result; isc_time_t now; isc_time_t loadtime, filetime; @@ -1599,12 +1599,14 @@ zone_load(dns_zone_t *zone, unsigned int flags) { REQUIRE(DNS_ZONE_VALID(zone)); - LOCK_ZONE(zone); + if (!locked) + LOCK_ZONE(zone); hasraw = inline_secure(zone); if (hasraw) { - result = zone_load(zone->raw, flags); + result = zone_load(zone->raw, flags, ISC_FALSE); if (result != ISC_R_SUCCESS) { - UNLOCK_ZONE(zone); + if (!locked) + UNLOCK_ZONE(zone); return(result); } LOCK_ZONE(zone->raw); @@ -1762,7 +1764,8 @@ zone_load(dns_zone_t *zone, unsigned int flags) { cleanup: if (hasraw) UNLOCK_ZONE(zone->raw); - UNLOCK_ZONE(zone); + if (!locked) + UNLOCK_ZONE(zone); if (db != NULL) dns_db_detach(&db); return (result); @@ -1770,12 +1773,12 @@ zone_load(dns_zone_t *zone, unsigned int flags) { isc_result_t dns_zone_load(dns_zone_t *zone) { - return (zone_load(zone, 0)); + return (zone_load(zone, 0, ISC_FALSE)); } isc_result_t dns_zone_loadnew(dns_zone_t *zone) { - return (zone_load(zone, DNS_ZONELOADFLAG_NOSTAT)); + return (zone_load(zone, DNS_ZONELOADFLAG_NOSTAT, ISC_FALSE)); } static void @@ -1783,6 +1786,7 @@ zone_asyncload(isc_task_t *task, isc_event_t *event) { dns_asyncload_t *asl = event->ev_arg; dns_zone_t *zone = asl->zone; isc_result_t result = ISC_R_SUCCESS; + isc_boolean_t load_pending; UNUSED(task); @@ -1791,13 +1795,21 @@ zone_asyncload(isc_task_t *task, isc_event_t *event) { if ((event->ev_attributes & ISC_EVENTATTR_CANCELED) != 0) result = ISC_R_CANCELED; isc_event_free(&event); - if (result == ISC_R_CANCELED || - !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING)) + + if (result == ISC_R_CANCELED) goto cleanup; - zone_load(zone, 0); - + /* Make sure load is still pending */ LOCK_ZONE(zone); + load_pending = ISC_TF(DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING)); + + if (!load_pending) { + UNLOCK_ZONE(zone); + goto cleanup; + } + + zone_load(zone, 0, ISC_TRUE); + DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_LOADPENDING); UNLOCK_ZONE(zone); @@ -1821,6 +1833,10 @@ dns_zone_asyncload(dns_zone_t *zone, dns_zt_zoneloaded_t done, void *arg) { if (zone->zmgr == NULL) return (ISC_R_FAILURE); + /* If we already have a load pending, stop now */ + if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_LOADPENDING)) + return (ISC_R_ALREADYRUNNING); + asl = isc_mem_get(zone->mctx, sizeof (*asl)); if (asl == NULL) CHECK(ISC_R_NOMEMORY); @@ -1862,9 +1878,10 @@ dns_zone_loadandthaw(dns_zone_t *zone) { isc_result_t result; if (inline_raw(zone)) - result = zone_load(zone->secure, DNS_ZONELOADFLAG_THAW); + result = zone_load(zone->secure, DNS_ZONELOADFLAG_THAW, + ISC_FALSE); else - result = zone_load(zone, DNS_ZONELOADFLAG_THAW); + result = zone_load(zone, DNS_ZONELOADFLAG_THAW, ISC_FALSE); switch (result) { case DNS_R_CONTINUE: