Do not detach raw zone until dumping is complete

When the signed version of an inline-signed zone is dumped to disk, the
serial number of the unsigned version of the zone is stored in the
raw-format header so that the contents of the signed zone can be
resynchronized after named restart if the unsigned zone file is modified
while named is not running.

In order for the serial number of the unsigned zone to be determined
during the dump, zone->raw must be set to a non-NULL value.  This should
always be the case as long as the signed version of the zone is used for
anything by named.

However, a scenario exists in which the signed version of the zone has
zone->raw set to NULL while it is being dumped:

 1. Zone dump is requested; zone_dump() is invoked.

 2. Another zone dump is already in progress, so the dump gets deferred
    until I/O is available (see zonemgr_getio()).

 3. The last external reference to the zone is released.
    zone_shutdown() gets queued to the zone's task.

 4. I/O becomes available for zone dumping.  zone_gotwritehandle() gets
    queued to the zone's task.

 5. The zone's task runs zone_shutdown().  zone->raw gets set to NULL.

 6. The zone's task runs zone_gotwritehandle().  zone->raw is determined
    to be NULL, causing the serial number of the unsigned version of the
    zone to be omitted from the raw-format dump of the signed zone file.

Note that the naïve solution - deferring the dns_zone_detach() call for
zone->raw until zone_free() gets called for the secure version of the
zone - does not work because it leads to a chicken-and-egg problem when
the inline-signed zone is about to get freed: the raw zone holds a weak
reference to the secure zone and that reference does not get released
until the reference count for the raw zone reaches zero, which in turn
would not happen until all weak references to the secure zone were
released.

Defer detaching from zone->raw in zone_shutdown() if the zone is in the
process of being dumped to disk.  Ensure zone->raw gets detached from
after the dump is finished if detaching gets deferred.  Prevent zone
dumping from being requeued upon failure if the zone is in the process
of being cleaned up as it opens up possibilities for the zone->raw
reference to leak, triggering a shutdown hang.

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index 2e54038b3cf6585bd0c9da08025192c96d958084..4c796351936005ec4c50b5cea9505367c28b9d2d 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -11925,7 +11925,22 @@ dump_done(void *arg, isc_result_t result) {
        if (compact) {
                DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_NEEDCOMPACT);
        }
-       if (result != ISC_R_SUCCESS && result != ISC_R_CANCELED) {
+       if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN)) {
+               /*
+                * If DNS_ZONEFLG_SHUTDOWN is set, all external references to
+                * the zone are gone, which means it is in the process of being
+                * cleaned up, so do not reschedule dumping.
+                *
+                * Detach from the raw version of the zone in case this
+                * operation has been deferred in zone_shutdown().
+                */
+               if (zone->raw != NULL) {
+                       dns_zone_detach(&zone->raw);
+               }
+               if (result == ISC_R_SUCCESS) {
+                       DNS_ZONE_CLRFLAG(zone, DNS_ZONEFLG_FLUSH);
+               }
+       } else if (result != ISC_R_SUCCESS && result != ISC_R_CANCELED) {
                /*
                 * Try again in a short while.
                 */
@@ -15028,7 +15043,13 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) {
         */
        DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_SHUTDOWN);
        free_needed = exit_check(zone);
-       if (inline_secure(zone)) {
+       /*
+        * If a dump is in progress for the secure zone, defer detaching from
+        * the raw zone as it may prevent the unsigned serial number from being
+        * stored in the raw-format dump of the secure zone.  In this scenario,
+        * dump_done() takes care of cleaning up the zone->raw reference.
+        */
+       if (inline_secure(zone) && !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_DUMPING)) {
                raw = zone->raw;
                zone->raw = NULL;
        }