From 656eed7c9bd1ee735777f4d2a11294b68e6e9e17 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Sat, 18 Nov 2017 07:11:12 +1100 Subject: [PATCH] 4821. [bug] When resigning ensure that the SOA's expire time is always later that the resigning time of other records. [RT #46473] 4820. [bug] dns_db_subtractrdataset should transfer the resigning information to the new header. [RT #46473] 4819. [bug] Fully backout the transaction when adding a RRset to the resigning / removal heaps fail. [RT #46473] --- CHANGES | 10 ++++ lib/dns/rbtdb.c | 121 ++++++++++++++++++++++++++++++------------------ lib/dns/zone.c | 2 +- 3 files changed, 87 insertions(+), 46 deletions(-) diff --git a/CHANGES b/CHANGES index 95aa26898d..8baa19b596 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,13 @@ +4821. [bug] When resigning ensure that the SOA's expire time is + always later that the resigning time of other records. + [RT #46473] + +4820. [bug] dns_db_subtractrdataset should transfer the resigning + information to the new header. [RT #46473] + +4819. [bug] Fully backout the transaction when adding a RRset + to the resigning / removal heaps fail. [RT #46473] + 4818. [test] The logfileconfig system test could intermittently report false negatives on some platforms. [RT #46615] diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c index 85dabd2578..cfeceda75d 100644 --- a/lib/dns/rbtdb.c +++ b/lib/dns/rbtdb.c @@ -6490,6 +6490,40 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, return (result); } } else { + idx = newheader->node->locknum; + if (IS_CACHE(rbtdb)) { + /* + * XXXMLG We don't check the return value + * here. If it fails, we will not do TTL + * based expiry on this node. However, we + * will do it on the LRU side, so memory + * will not leak... for long. + */ + INSIST(rbtdb->heaps != NULL); + result = isc_heap_insert(rbtdb->heaps[idx], + newheader); + if (result != ISC_R_SUCCESS) { + free_rdataset(rbtdb, + rbtdb->common.mctx, + newheader); + return (result); + } + if (ZEROTTL(newheader)) + ISC_LIST_APPEND(rbtdb->rdatasets[idx], + newheader, link); + else + ISC_LIST_PREPEND(rbtdb->rdatasets[idx], + newheader, link); + } else if (RESIGN(newheader)) { + result = resign_insert(rbtdb, idx, newheader); + if (result != ISC_R_SUCCESS) { + free_rdataset(rbtdb, + rbtdb->common.mctx, + newheader); + return (result); + } + resign_delete(rbtdb, rbtversion, header); + } newheader->down = topheader; topheader->next = newheader; rbtnode->dirty = 1; @@ -6503,30 +6537,6 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, mark_header_ancient(rbtdb, sigheader); } } - idx = newheader->node->locknum; - if (IS_CACHE(rbtdb)) { - if (ZEROTTL(newheader)) - ISC_LIST_APPEND(rbtdb->rdatasets[idx], - newheader, link); - else - ISC_LIST_PREPEND(rbtdb->rdatasets[idx], - newheader, link); - /* - * XXXMLG We don't check the return value - * here. If it fails, we will not do TTL - * based expiry on this node. However, we - * will do it on the LRU side, so memory - * will not leak... for long. - */ - INSIST(rbtdb->heaps != NULL); - (void)isc_heap_insert(rbtdb->heaps[idx], - newheader); - } else if (RESIGN(newheader)) { - resign_delete(rbtdb, rbtversion, header); - result = resign_insert(rbtdb, idx, newheader); - if (result != ISC_R_SUCCESS) - return (result); - } } } else { /* @@ -6542,6 +6552,30 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, return (DNS_R_UNCHANGED); } + idx = newheader->node->locknum; + if (IS_CACHE(rbtdb)) { + result = isc_heap_insert(rbtdb->heaps[idx], newheader); + if (result != ISC_R_SUCCESS) { + free_rdataset(rbtdb, rbtdb->common.mctx, + newheader); + return (result); + } + if (ZEROTTL(newheader)) + ISC_LIST_APPEND(rbtdb->rdatasets[idx], + newheader, link); + else + ISC_LIST_PREPEND(rbtdb->rdatasets[idx], + newheader, link); + } else if (RESIGN(newheader)) { + result = resign_insert(rbtdb, idx, newheader); + if (result != ISC_R_SUCCESS) { + free_rdataset(rbtdb, rbtdb->common.mctx, + newheader); + return (result); + } + resign_delete(rbtdb, rbtversion, header); + } + if (topheader != NULL) { /* * We have an list of rdatasets of the given type, @@ -6582,21 +6616,6 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion, sizeof(*newheader)); RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write); } - idx = newheader->node->locknum; - if (IS_CACHE(rbtdb)) { - if (ZEROTTL(newheader)) - ISC_LIST_APPEND(rbtdb->rdatasets[idx], - newheader, link); - else - ISC_LIST_PREPEND(rbtdb->rdatasets[idx], - newheader, link); - isc_heap_insert(rbtdb->heaps[idx], newheader); - } else if (RESIGN(newheader)) { - resign_delete(rbtdb, rbtversion, header); - result = resign_insert(rbtdb, idx, newheader); - if (result != ISC_R_SUCCESS) - return (result); - } } /* @@ -7041,6 +7060,19 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version, newheader = (rdatasetheader_t *)subresult; init_rdataset(rbtdb, newheader); update_newheader(newheader, header); + if (RESIGN(header)) { + newheader->attributes |= RDATASET_ATTR_RESIGN; + newheader->resign = header->resign; + newheader->resign_lsb = header->resign_lsb; + result = resign_insert(rbtdb, rbtnode->locknum, + newheader); + if (result != ISC_R_SUCCESS) { + free_rdataset(rbtdb, + rbtdb->common.mctx, + newheader); + goto unlock; + } + } /* * We have to set the serial since the rdataslab * subtraction routine copies the reserved portion of @@ -8066,9 +8098,8 @@ getsize(dns_db_t *db, dns_dbversion_t *version, isc_uint64_t *records, static isc_result_t setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)db; - isc_stdtime_t oldresign; isc_result_t result = ISC_R_SUCCESS; - rdatasetheader_t *header; + rdatasetheader_t *header, oldheader; REQUIRE(VALID_RBTDB(rbtdb)); REQUIRE(!IS_CACHE(rbtdb)); @@ -8080,7 +8111,7 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { NODE_LOCK(&rbtdb->node_locks[header->node->locknum].lock, isc_rwlocktype_write); - oldresign = (header->resign << 1) | header->resign_lsb; + oldheader = *header; header->resign = (isc_stdtime_t)(dns_time64_from32(resign) >> 1); header->resign_lsb = resign & 0x1; if (header->heap_index != 0) { @@ -8089,13 +8120,13 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) { isc_heap_delete(rbtdb->heaps[header->node->locknum], header->heap_index); header->heap_index = 0; - } else if (resign < oldresign) + } else if (resign_sooner(header, &oldheader)) isc_heap_increased(rbtdb->heaps[header->node->locknum], header->heap_index); - else if (resign > oldresign) + else if (resign_sooner(&oldheader, header)) isc_heap_decreased(rbtdb->heaps[header->node->locknum], header->heap_index); - } else if (resign && header->heap_index == 0) { + } else if (resign != 0 && header->heap_index == 0) { header->attributes |= RDATASET_ATTR_RESIGN; result = resign_insert(rbtdb, header->node->locknum, header); } diff --git a/lib/dns/zone.c b/lib/dns/zone.c index dab209cbb3..708c119b56 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -6533,7 +6533,7 @@ zone_resigninc(dns_zone_t *zone) { * we still want some clustering to occur. */ isc_random_get(&jitter); - expire = soaexpire - jitter % 3600; + expire = soaexpire - jitter % 3600 - 1; stop = now + 5; check_ksk = DNS_ZONE_OPTION(zone, DNS_ZONEOPT_UPDATECHECKKSK);