4819.   [bug]           Fully backout the transaction when adding a RRset
                        to the resigning / removal heaps fails. [RT #46473]
This commit is contained in:
Mark Andrews 2017-11-27 15:15:41 +11:00
parent 14e9925868
commit 19f6a63184
4 changed files with 116 additions and 82 deletions

38
OPTIONS
View file

@ -4,25 +4,21 @@ defined in configure.
Some of these settings are:
Setting Description
Overwrite memory with tag values when allocating
-DISC_MEM_DEFAULTFILL=1 or freeing it; this impairs performance but
makes debugging of memory problems easier.
Don't track memory allocations by file and line
-DISC_MEM_TRACKLINES=0 number; this improves performance but makes
debugging more difficult.
-DISC_FACILITY=LOG_LOCAL0 Change the default syslog facility for named
-DNS_CLIENT_DROPPORT=0 Disable dropping queries from particular
well-known ports:
-DCHECK_SIBLING=0 Don't check sibling glue in named-checkzone
-DCHECK_LOCAL=0 Don't check out-of-zone addresses in
named-checkzone
-DNS_RUN_PID_DIR=0 Create default PID files in ${localstatedir}/run
rather than ${localstatedir}/run/named/
Increase the maximum number of configurable
-DNS_RPZ_MAX_ZONES=64 response policy zones from 32 to 64; this is the
highest possible setting
Disable the use of inline functions to implement
-DISC_BUFFER_USEINLINE=0 the isc_buffer API: this reduces performance but
may be useful when debugging
|Setting |Description | |-----------------------------------|
----------------------------------------| |-DISC_MEM_DEFAULTFILL=1|
Overwrite memory with tag values when allocating or freeing it; this
impairs performance but makes debugging of memory problems easier.| |
-DISC_MEM_TRACKLINES=0|Don't track memory allocations by file and line
number; this improves performance but makes debugging more difficult.| |
-DISC_FACILITY=LOG_LOCAL0|Change the default syslog facility for named| |
-DNS_CLIENT_DROPPORT=0|Disable dropping queries from particular well-known
ports:| |-DCHECK_SIBLING=0|Don't check sibling glue in named-checkzone| |
-DCHECK_LOCAL=0|Don't check out-of-zone addresses in named-checkzone| |
-DNS_RUN_PID_DIR=0|Create default PID files in ${localstatedir}/run rather
than ${localstatedir}/run/named/| |-DNS_RPZ_MAX_ZONES=64|Increase the
maximum number of configurable response policy zones from 32 to 64; this
is the highest possible setting| |-DISC_BUFFER_USEINLINE=0|Disable the use
of inline functions to implement the isc_buffer API: this reduces
performance but may be useful when debugging | |-DISC_HEAP_CHECK|Test heap
consistency after every heap operation; used when debugging |

View file

@ -22,3 +22,5 @@ Some of these settings are:
|`-DNS_RUN_PID_DIR=0`|Create default PID files in `${localstatedir}/run` rather than `${localstatedir}/run/named/`|
|`-DNS_RPZ_MAX_ZONES=64`|Increase the maximum number of configurable response policy zones from 32 to 64; this is the highest possible setting|
|`-DISC_BUFFER_USEINLINE=0`|Disable the use of inline functions to implement the `isc_buffer` API: this reduces performance but may be useful when debugging |
|`-DISC_HEAP_CHECK`|Test heap consistency after every heap operation; used
when debugging |

View file

@ -6100,6 +6100,22 @@ resign_delete(dns_rbtdb_t *rbtdb, rbtdb_version_t *version,
}
}
static void
update_recordsandbytes(isc_boolean_t add, rbtdb_version_t *rbtversion,
rdatasetheader_t *header)
{
unsigned char *hdr = (unsigned char *)header;
size_t hdrsize = sizeof (*header);
if (add) {
rbtversion->records += dns_rdataslab_count(hdr, hdrsize);
rbtversion->bytes += dns_rdataslab_size(hdr, hdrsize);
} else {
rbtversion->records -= dns_rdataslab_count(hdr, hdrsize);
rbtversion->bytes -= dns_rdataslab_size(hdr, hdrsize);
}
}
static isc_result_t
add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
rdatasetheader_t *newheader, unsigned int options, isc_boolean_t loading,
@ -6438,41 +6454,8 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
}
INSIST(rbtversion == NULL ||
rbtversion->serial >= topheader->serial);
if (topheader_prev != NULL)
topheader_prev->next = newheader;
else
rbtnode->data = newheader;
newheader->next = topheader->next;
if (rbtversion != NULL)
RWLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
if (rbtversion != NULL && !header_nx) {
rbtversion->records -=
dns_rdataslab_count((unsigned char *)header,
sizeof(*header));
rbtversion->bytes -=
dns_rdataslab_size((unsigned char *)header,
sizeof(*header));
}
if (rbtversion != NULL && !newheader_nx) {
rbtversion->records +=
dns_rdataslab_count((unsigned char *)newheader,
sizeof(*newheader));
rbtversion->bytes +=
dns_rdataslab_size((unsigned char *)newheader,
sizeof(*newheader));
}
if (rbtversion != NULL)
RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
if (loading) {
/*
* There are no other references to 'header' when
* loading, so we MAY clean up 'header' now.
* Since we don't generate changed records when
* loading, we MUST clean up 'header' now.
*/
newheader->down = NULL;
free_rdataset(rbtdb, rbtdb->common.mctx, header);
idx = newheader->node->locknum;
if (IS_CACHE(rbtdb)) {
if (ZEROTTL(newheader))
@ -6482,13 +6465,49 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
ISC_LIST_PREPEND(rbtdb->rdatasets[idx],
newheader, link);
INSIST(rbtdb->heaps != NULL);
(void)isc_heap_insert(rbtdb->heaps[idx],
result = isc_heap_insert(rbtdb->heaps[idx],
newheader);
if (result != ISC_R_SUCCESS) {
free_rdataset(rbtdb,
rbtdb->common.mctx,
newheader);
return (result);
}
} else if (RESIGN(newheader)) {
result = resign_insert(rbtdb, idx, newheader);
if (result != ISC_R_SUCCESS)
if (result != ISC_R_SUCCESS) {
free_rdataset(rbtdb,
rbtdb->common.mctx,
newheader);
return (result);
}
/*
* Don't call resign_delete as we don't need
* to reverse the delete. The free_rdataset
* call below will clean up the heap entry.
*/
}
/*
* There are no other references to 'header' when
* loading, so we MAY clean up 'header' now.
* Since we don't generate changed records when
* loading, we MUST clean up 'header' now.
*/
if (topheader_prev != NULL)
topheader_prev->next = newheader;
else
rbtnode->data = newheader;
newheader->next = topheader->next;
if (rbtversion != NULL && !header_nx) {
RWLOCK(&rbtversion->rwlock,
isc_rwlocktype_write);
update_recordsandbytes(ISC_FALSE, rbtversion,
header);
RWUNLOCK(&rbtversion->rwlock,
isc_rwlocktype_write);
}
free_rdataset(rbtdb, rbtdb->common.mctx, header);
} else {
idx = newheader->node->locknum;
if (IS_CACHE(rbtdb)) {
@ -6517,6 +6536,11 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
}
resign_delete(rbtdb, rbtversion, header);
}
if (topheader_prev != NULL)
topheader_prev->next = newheader;
else
rbtnode->data = newheader;
newheader->next = topheader->next;
newheader->down = topheader;
topheader->next = newheader;
rbtnode->dirty = 1;
@ -6530,6 +6554,14 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
mark_header_ancient(rbtdb, sigheader);
}
}
if (rbtversion != NULL && !header_nx) {
RWLOCK(&rbtversion->rwlock,
isc_rwlocktype_write);
update_recordsandbytes(ISC_FALSE, rbtversion,
header);
RWUNLOCK(&rbtversion->rwlock,
isc_rwlocktype_write);
}
}
} else {
/*
@ -6599,16 +6631,12 @@ add32(dns_rbtdb_t *rbtdb, dns_rbtnode_t *rbtnode, rbtdb_version_t *rbtversion,
newheader->down = NULL;
rbtnode->data = newheader;
}
if (rbtversion != NULL && !newheader_nx) {
RWLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
rbtversion->records +=
dns_rdataslab_count((unsigned char *)newheader,
sizeof(*newheader));
rbtversion->bytes +=
dns_rdataslab_size((unsigned char *)newheader,
sizeof(*newheader));
RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
}
}
if (rbtversion != NULL && !newheader_nx) {
RWLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
update_recordsandbytes(ISC_TRUE, rbtversion, newheader);
RWUNLOCK(&rbtversion->rwlock, isc_rwlocktype_write);
}
/*
@ -7077,12 +7105,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
* to additional info. We need to clear these fields
* to avoid having duplicated references.
*/
rbtversion->records +=
dns_rdataslab_count((unsigned char *)newheader,
sizeof(*newheader));
rbtversion->bytes +=
dns_rdataslab_size((unsigned char *)newheader,
sizeof(*newheader));
update_recordsandbytes(ISC_TRUE, rbtversion, newheader);
} else if (result == DNS_R_NXRRSET) {
/*
* This subtraction would remove all of the rdata;
@ -7117,12 +7140,7 @@ subtractrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
* topheader.
*/
INSIST(rbtversion->serial >= topheader->serial);
rbtversion->records -=
dns_rdataslab_count((unsigned char *)header,
sizeof(*header));
rbtversion->bytes -=
dns_rdataslab_size((unsigned char *)header,
sizeof(*header));
update_recordsandbytes(ISC_FALSE, rbtversion, header);
if (topheader_prev != NULL)
topheader_prev->next = newheader;
else
@ -8120,14 +8138,14 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
if (resign == 0) {
isc_heap_delete(rbtdb->heaps[header->node->locknum],
header->heap_index);
header->heap_index = 0;
} else if (resign_sooner(header, &oldheader))
} else if (resign_sooner(header, &oldheader)) {
isc_heap_increased(rbtdb->heaps[header->node->locknum],
header->heap_index);
else if (resign_sooner(&oldheader, header))
} else if (resign_sooner(&oldheader, header)) {
isc_heap_decreased(rbtdb->heaps[header->node->locknum],
header->heap_index);
} else if (resign != 0 && header->heap_index == 0) {
}
} else if (resign != 0) {
header->attributes |= RDATASET_ATTR_RESIGN;
result = resign_insert(rbtdb, header->node->locknum, header);
}

View file

@ -63,6 +63,18 @@ struct isc_heap {
isc_heapindex_t index;
};
#ifdef ISC_HEAP_CHECK
static void
heap_check(isc_heap_t *heap) {
unsigned int i;
for (i = 1; i <= heap->last; i++) {
INSIST(HEAPCONDITION(i));
}
}
#else
#define heap_check(x) (void)0
#endif
isc_result_t
isc_heap_create(isc_mem_t *mctx, isc_heapcompare_t compare,
isc_heapindex_t idx, unsigned int size_increment,
@ -149,6 +161,7 @@ float_up(isc_heap_t *heap, unsigned int i, void *elt) {
(heap->index)(heap->array[i], i);
INSIST(HEAPCONDITION(i));
heap_check(heap);
}
static void
@ -174,6 +187,7 @@ sink_down(isc_heap_t *heap, unsigned int i, void *elt) {
(heap->index)(heap->array[i], i);
INSIST(HEAPCONDITION(i));
heap_check(heap);
}
isc_result_t
@ -182,6 +196,7 @@ isc_heap_insert(isc_heap_t *heap, void *elt) {
REQUIRE(VALID_HEAP(heap));
heap_check(heap);
new_last = heap->last + 1;
RUNTIME_CHECK(new_last > 0); /* overflow check */
if (new_last >= heap->size && !resize(heap))
@ -201,9 +216,11 @@ isc_heap_delete(isc_heap_t *heap, unsigned int idx) {
REQUIRE(VALID_HEAP(heap));
REQUIRE(idx >= 1 && idx <= heap->last);
heap_check(heap);
if (idx == heap->last) {
heap->array[heap->last] = NULL;
heap->last--;
heap_check(heap);
} else {
elt = heap->array[heap->last];
heap->array[heap->last] = NULL;
@ -239,6 +256,7 @@ isc_heap_element(isc_heap_t *heap, unsigned int idx) {
REQUIRE(VALID_HEAP(heap));
REQUIRE(idx >= 1);
heap_check(heap);
if (idx <= heap->last)
return (heap->array[idx]);
return (NULL);