Fix rss_state leak in receive_secure_serial_cancel

Previously receive_secure_serial_cancel() could leak .rss_state on
inline secure bootstrap or zone shutdown, if a pre-existing DNSSEC
signing pass was paused. This has been fixed.

diff --git a/lib/dns/include/dns/update.h b/lib/dns/include/dns/update.h
index b0fdf17fb52eddd01a238a88055359785e412306..306e783c372f35cec2d1f39ae6f5cd5d395d5002 100644 (file)
--- a/lib/dns/include/dns/update.h
+++ b/lib/dns/include/dns/update.h
@@ -66,4 +66,4 @@ dns_update_signaturesinc(dns_update_log_t *log, dns_zone_t *zone, dns_db_t *db,
                         dns_update_state_t **statep);
 
 void
-dns_update_state_clear(dns_update_state_t **statep, bool destroy);
+dns_update_state_clear(dns_update_state_t **statep);

diff --git a/lib/dns/update.c b/lib/dns/update.c
index 276a06f762798b9210f8a2deeed78dc4481f3afa..528ccd2898f505b753797875802ecbd94eac9c72 100644 (file)
--- a/lib/dns/update.c
+++ b/lib/dns/update.c
@@ -1334,6 +1334,23 @@ struct dns_update_state {
        } state;
 };
 
+static void
+dns_update_state_clear_contents(dns_update_state_t *state) {
+       REQUIRE(DNS_STATE_VALID(state));
+
+       dns_diff_clear(&state->sig_diff);
+       dns_diff_clear(&state->nsec_diff);
+       dns_diff_clear(&state->nsec_mindiff);
+
+       dns_diff_clear(&state->affected);
+       dns_diff_clear(&state->diffnames);
+       dns_diff_clear(&state->work);
+
+       for (size_t i = 0; i < state->nkeys; i++) {
+               dst_key_free(&state->zone_keys[i]);
+       }
+}
+
 static uint32_t
 dns__jitter_expire(dns_zone_t *zone) {
        /* Spread out signatures over time */
@@ -2007,35 +2024,31 @@ cleanup:
                dns_db_detachnode(&node);
        }
 
-       dns_update_state_clear(&state, state != &mystate);
+       if (state == &mystate) {
+               dns_update_state_clear_contents(state);
+       } else {
+               dns_update_state_t *state_to_clear = state;
+               dns_update_state_clear(&state_to_clear);
+       }
        SET_IF_NOT_NULL(statep, NULL);
 
        return result;
 }
 
 void
-dns_update_state_clear(dns_update_state_t **statep, bool destroy) {
-       REQUIRE(DNS_STATE_VALID(*statep));
-
-       dns_update_state_t *state = *statep;
-
-       dns_diff_clear(&state->sig_diff);
-       dns_diff_clear(&state->nsec_diff);
-       dns_diff_clear(&state->nsec_mindiff);
-
-       dns_diff_clear(&state->affected);
-       dns_diff_clear(&state->diffnames);
-       dns_diff_clear(&state->work);
+dns_update_state_clear(dns_update_state_t **statep) {
+       dns_update_state_t *state = NULL;
 
-       for (size_t i = 0; i < state->nkeys; i++) {
-               dst_key_free(&state->zone_keys[i]);
+       if (statep == NULL || *statep == NULL) {
+               return;
        }
 
-       if (destroy) {
-               *statep = NULL;
-               state->magic = 0;
-               isc_mem_putanddetach(&state->mctx, state, sizeof(*state));
-       }
+       state = *statep;
+       dns_update_state_clear_contents(state);
+
+       *statep = NULL;
+       state->magic = 0;
+       isc_mem_putanddetach(&state->mctx, state, sizeof(*state));
 }
 
 static isc_stdtime_t

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index ddcca84e6b49f8a31d047b46113df11a452997ee..8835ba0e9d20a0c628370937a915429a1c642893 100644 (file)
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -616,7 +616,7 @@ dns__zone_free(dns_zone_t *zone) {
        }
 
        if (zone->rss_state != NULL) {
-               dns_update_state_clear(&zone->rss_state, true);
+               dns_update_state_clear(&zone->rss_state);
        }
 
        if (zone->masterfile != NULL) {
@@ -14388,11 +14388,12 @@ receive_secure_serial_cancel(dns_zone_t *zone) {
        dns_db_t *rss_db = NULL;
        dns_dbversion_t *rss_oldver = NULL;
        dns_dbversion_t *rss_newver = NULL;
+       dns_update_state_t *rss_state = NULL;
 
        REQUIRE(DNS_ZONE_VALID(zone));
 
        LOCK_ZONE(zone);
-       if (zone->rss_zone == NULL) {
+       if (zone->rss_zone == NULL && zone->rss_state == NULL) {
                UNLOCK_ZONE(zone);
                return;
        }
@@ -14406,10 +14407,12 @@ receive_secure_serial_cancel(dns_zone_t *zone) {
        rss_db = MOVE_OWNERSHIP(zone->rss_db);
        rss_oldver = MOVE_OWNERSHIP(zone->rss_oldver);
        rss_newver = MOVE_OWNERSHIP(zone->rss_newver);
+       rss_state = MOVE_OWNERSHIP(zone->rss_state);
        zone->rss_end = 0;
        dns_diff_clear(&zone->rss_diff);
        UNLOCK_ZONE(zone);
 
+       dns_update_state_clear(&rss_state);
        if (rss_db != NULL) {
                if (rss_oldver != NULL) {
                        dns_db_closeversion(rss_db, &rss_oldver, false);
@@ -14422,7 +14425,9 @@ receive_secure_serial_cancel(dns_zone_t *zone) {
        if (rss_raw != NULL) {
                dns_zone_detach(&rss_raw);
        }
-       dns_zone_idetach(&rss_zone);
+       if (rss_zone != NULL) {
+               dns_zone_idetach(&rss_zone);
+       }
 }
 
 static void