mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-30 22:15:20 +00:00
Code Issues Releases Wiki Activity

Merge branch 'each-rbtdb-dbiterator-fixes' into 'main'

Browse Source 
fix several bugs in the RBTDB dbiterator implementation

See merge request isc-projects/bind9!8741
This commit is contained in:
Evan Hunt
2024-02-15 18:52:47 +00:00
parent 88c56e25a1 32241022b5
commit a2dc8ebcc0
5 changed files with 210 additions and 35 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
CHANGES
View File

@@ -1,3 +1,6 @@
6346. [bug] Cleaned up several minor bugs in the RBTDB dbiterator
implementation. [GL !8741]
6345. [bug] Added missing dns_rdataset_disassociate calls in
validator.c:findnsec3proofs. [GL #4571]

2
lib/dns/db.c
View File

@@ -608,6 +608,8 @@ dns_db_createiterator(dns_db_t *db, unsigned int flags,
REQUIRE(DNS_DB_VALID(db));
REQUIRE(iteratorp != NULL && *iteratorp == NULL);
REQUIRE((flags & (DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3)) !=
(DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3));
if (db->methods->createiterator != NULL) {
return (db->methods->createiterator(db, flags, iteratorp));

2
lib/dns/include/dns/db.h
View File

@@ -1089,6 +1089,8 @@ dns_db_createiterator(dns_db_t *db, unsigned int options,
*
* \li iteratorp != NULL && *iteratorp == NULL
*
* \li 'flags' contains at most one of #DNS_DB_NSEC3ONLY and #DNS_DB_NONSEC3.
*
* Ensures:
*
* \li On success, *iteratorp will be a valid database iterator.

156
lib/dns/rbtdb.c
View File

@@ -130,6 +130,10 @@
#define KEEPSTALE(rbtdb) ((rbtdb)->common.serve_stale_ttl > 0)
#define RBTDBITER_NSEC3_ORIGIN_NODE(rbtdb, iterator) \
((iterator)->current == &(iterator)->nsec3chain && \
(iterator)->node == (rbtdb)->nsec3_origin_node)
/*%
* Number of buckets for cache DB entries (locks, LRU lists, TTL heaps).
* There is a tradeoff issue about configuring this value: if this is too
@@ -228,8 +232,7 @@ typedef struct rbtdb_dbiterator {
dns_rbtnodechain_t nsec3chain;
dns_rbtnodechain_t *current;
dns_rbtnode_t *node;
bool nsec3only;
bool nonsec3;
enum { full, nonsec3, nsec3only } nsec3mode;
} rbtdb_dbiterator_t;
static void
@@ -2340,6 +2343,8 @@ dns__rbtdb_createiterator(dns_db_t *db, unsigned int options,
rbtdb_dbiterator_t *rbtdbiter = NULL;
REQUIRE(VALID_RBTDB(rbtdb));
REQUIRE((options & (DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3)) !=
(DNS_DB_NSEC3ONLY | DNS_DB_NONSEC3));
rbtdbiter = isc_mem_get(rbtdb->common.mctx, sizeof(*rbtdbiter));
@@ -2355,11 +2360,16 @@ dns__rbtdb_createiterator(dns_db_t *db, unsigned int options,
dns_fixedname_init(&rbtdbiter->name);
dns_fixedname_init(&rbtdbiter->origin);
rbtdbiter->node = NULL;
rbtdbiter->nsec3only = ((options & DNS_DB_NSEC3ONLY) != 0);
rbtdbiter->nonsec3 = ((options & DNS_DB_NONSEC3) != 0);
if ((options & DNS_DB_NSEC3ONLY) != 0) {
rbtdbiter->nsec3mode = nsec3only;
} else if ((options & DNS_DB_NONSEC3) != 0) {
rbtdbiter->nsec3mode = nonsec3;
} else {
rbtdbiter->nsec3mode = full;
}
dns_rbtnodechain_init(&rbtdbiter->chain);
dns_rbtnodechain_init(&rbtdbiter->nsec3chain);
if (rbtdbiter->nsec3only) {
if (rbtdbiter->nsec3mode == nsec3only) {
rbtdbiter->current = &rbtdbiter->nsec3chain;
} else {
rbtdbiter->current = &rbtdbiter->chain;
@@ -4387,23 +4397,48 @@ dbiterator_first(dns_dbiterator_t *iterator DNS__DB_FLARG) {
dns_rbtnodechain_reset(&rbtdbiter->chain);
dns_rbtnodechain_reset(&rbtdbiter->nsec3chain);
if (rbtdbiter->nsec3only) {
switch (rbtdbiter->nsec3mode) {
case nsec3only:
rbtdbiter->current = &rbtdbiter->nsec3chain;
result = dns_rbtnodechain_first(rbtdbiter->current,
rbtdb->nsec3, name, origin);
} else {
break;
case nonsec3:
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbtnodechain_first(rbtdbiter->current, rbtdb->tree,
name, origin);
if (!rbtdbiter->nonsec3 && result == ISC_R_NOTFOUND) {
break;
case full:
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbtnodechain_first(rbtdbiter->current, rbtdb->tree,
name, origin);
if (result == ISC_R_NOTFOUND) {
rbtdbiter->current = &rbtdbiter->nsec3chain;
result = dns_rbtnodechain_first(
rbtdbiter->current, rbtdb->nsec3, name, origin);
}
break;
default:
UNREACHABLE();
}
if (result == ISC_R_SUCCESS || result == DNS_R_NEWORIGIN) {
result = dns_rbtnodechain_current(rbtdbiter->current, NULL,
NULL, &rbtdbiter->node);
/* If we're in the NSEC3 tree, skip the origin */
if (RBTDBITER_NSEC3_ORIGIN_NODE(rbtdb, rbtdbiter)) {
rbtdbiter->node = NULL;
result = dns_rbtnodechain_next(rbtdbiter->current, name,
origin);
if (result == ISC_R_SUCCESS ||
result == DNS_R_NEWORIGIN)
{
result = dns_rbtnodechain_current(
rbtdbiter->current, NULL, NULL,
&rbtdbiter->node);
}
}
if (result == ISC_R_SUCCESS) {
rbtdbiter->new_origin = true;
reference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);
@@ -4448,20 +4483,61 @@ dbiterator_last(dns_dbiterator_t *iterator DNS__DB_FLARG) {
dns_rbtnodechain_reset(&rbtdbiter->chain);
dns_rbtnodechain_reset(&rbtdbiter->nsec3chain);
result = ISC_R_NOTFOUND;
if (rbtdbiter->nsec3only && !rbtdbiter->nonsec3) {
switch (rbtdbiter->nsec3mode) {
case nsec3only:
rbtdbiter->current = &rbtdbiter->nsec3chain;
result = dns_rbtnodechain_last(rbtdbiter->current, rbtdb->nsec3,
name, origin);
}
if (!rbtdbiter->nsec3only && result == ISC_R_NOTFOUND) {
break;
case nonsec3:
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbtnodechain_last(rbtdbiter->current, rbtdb->tree,
name, origin);
break;
case full:
rbtdbiter->current = &rbtdbiter->nsec3chain;
result = dns_rbtnodechain_last(rbtdbiter->current, rbtdb->nsec3,
name, origin);
if (result == ISC_R_NOTFOUND) {
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbtnodechain_last(
rbtdbiter->current, rbtdb->tree, name, origin);
}
break;
default:
UNREACHABLE();
}
if (result == ISC_R_SUCCESS || result == DNS_R_NEWORIGIN) {
result = dns_rbtnodechain_current(rbtdbiter->current, NULL,
NULL, &rbtdbiter->node);
if (RBTDBITER_NSEC3_ORIGIN_NODE(rbtdb, rbtdbiter)) {
/*
* NSEC3 tree only has an origin node.
*/
rbtdbiter->node = NULL;
switch (rbtdbiter->nsec3mode) {
case nsec3only:
result = ISC_R_NOMORE;
break;
case nonsec3:
case full:
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbtnodechain_last(
rbtdbiter->current, rbtdb->tree, name,
origin);
if (result == ISC_R_SUCCESS ||
result == DNS_R_NEWORIGIN)
{
result = dns_rbtnodechain_current(
rbtdbiter->current, NULL, NULL,
&rbtdbiter->node);
}
break;
default:
UNREACHABLE();
}
}
if (result == ISC_R_SUCCESS) {
rbtdbiter->new_origin = true;
reference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);
@@ -4503,17 +4579,20 @@ dbiterator_seek(dns_dbiterator_t *iterator,
dns_rbtnodechain_reset(&rbtdbiter->chain);
dns_rbtnodechain_reset(&rbtdbiter->nsec3chain);
if (rbtdbiter->nsec3only) {
switch (rbtdbiter->nsec3mode) {
case nsec3only:
rbtdbiter->current = &rbtdbiter->nsec3chain;
result = dns_rbt_findnode(rbtdb->nsec3, name, NULL,
&rbtdbiter->node, rbtdbiter->current,
DNS_RBTFIND_EMPTYDATA, NULL, NULL);
} else if (rbtdbiter->nonsec3) {
break;
case nonsec3:
rbtdbiter->current = &rbtdbiter->chain;
result = dns_rbt_findnode(rbtdb->tree, name, NULL,
&rbtdbiter->node, rbtdbiter->current,
DNS_RBTFIND_EMPTYDATA, NULL, NULL);
} else {
break;
case full:
/*
* Stay on main chain if not found on either chain.
*/
@@ -4533,6 +4612,9 @@ dbiterator_seek(dns_dbiterator_t *iterator,
result = tresult;
}
}
break;
default:
UNREACHABLE();
}
if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) {
@@ -4572,11 +4654,29 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
resume_iteration(rbtdbiter);
}
dereference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);
name = dns_fixedname_name(&rbtdbiter->name);
origin = dns_fixedname_name(&rbtdbiter->origin);
result = dns_rbtnodechain_prev(rbtdbiter->current, name, origin);
if (result == ISC_R_NOMORE && !rbtdbiter->nsec3only &&
!rbtdbiter->nonsec3 && &rbtdbiter->nsec3chain == rbtdbiter->current)
if (rbtdbiter->current == &rbtdbiter->nsec3chain &&
(result == ISC_R_SUCCESS || result == DNS_R_NEWORIGIN))
{
/*
* If we're in the NSEC3 tree, it's empty or we've
* reached the origin, then we're done with it.
*/
result = dns_rbtnodechain_current(rbtdbiter->current, NULL,
NULL, &rbtdbiter->node);
if (result == ISC_R_NOTFOUND ||
RBTDBITER_NSEC3_ORIGIN_NODE(rbtdb, rbtdbiter))
{
rbtdbiter->node = NULL;
result = ISC_R_NOMORE;
}
}
if (result == ISC_R_NOMORE && rbtdbiter->nsec3mode != nsec3only &&
&rbtdbiter->nsec3chain == rbtdbiter->current)
{
rbtdbiter->current = &rbtdbiter->chain;
dns_rbtnodechain_reset(rbtdbiter->current);
@@ -4587,8 +4687,6 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
}
}
dereference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);
if (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) {
rbtdbiter->new_origin = (result == DNS_R_NEWORIGIN);
result = dns_rbtnodechain_current(rbtdbiter->current, NULL,
@@ -4624,8 +4722,8 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
name = dns_fixedname_name(&rbtdbiter->name);
origin = dns_fixedname_name(&rbtdbiter->origin);
result = dns_rbtnodechain_next(rbtdbiter->current, name, origin);
if (result == ISC_R_NOMORE && !rbtdbiter->nsec3only &&
!rbtdbiter->nonsec3 && &rbtdbiter->chain == rbtdbiter->current)
if (result == ISC_R_NOMORE && rbtdbiter->nsec3mode != nonsec3 &&
&rbtdbiter->chain == rbtdbiter->current)
{
rbtdbiter->current = &rbtdbiter->nsec3chain;
dns_rbtnodechain_reset(rbtdbiter->current);
@@ -4639,9 +4737,25 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
dereference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);
if (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) {
/*
* If we've just started the NSEC3 tree,
* skip over the origin.
*/
rbtdbiter->new_origin = (result == DNS_R_NEWORIGIN);
result = dns_rbtnodechain_current(rbtdbiter->current, NULL,
NULL, &rbtdbiter->node);
if (RBTDBITER_NSEC3_ORIGIN_NODE(rbtdb, rbtdbiter)) {
rbtdbiter->node = NULL;
result = dns_rbtnodechain_next(rbtdbiter->current, name,
origin);
if (result == ISC_R_SUCCESS ||
result == DNS_R_NEWORIGIN)
{
result = dns_rbtnodechain_current(
rbtdbiter->current, NULL, NULL,
&rbtdbiter->node);
}
}
}
if (result == ISC_R_SUCCESS) {
reference_iter_node(rbtdbiter DNS__DB_FLARG_PASS);

82
tests/dns/dbiterator_test.c
View File

@@ -74,7 +74,7 @@ ISC_RUN_TEST_IMPL(create_nsec3) {
/* walk: walk a database */
static void
test_walk(const char *filename, int nodes) {
test_walk(const char *filename, int flags, int nodes) {
isc_result_t result;
dns_db_t *db = NULL;
dns_dbiterator_t *iter = NULL;
@@ -88,7 +88,7 @@ test_walk(const char *filename, int nodes) {
result = dns_test_loaddb(&db, dns_dbtype_zone, TEST_ORIGIN, filename);
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_db_createiterator(db, 0, &iter);
result = dns_db_createiterator(db, flags, &iter);
assert_int_equal(result, ISC_R_SUCCESS);
for (result = dns_dbiterator_first(iter); result == ISC_R_SUCCESS;
@@ -112,18 +112,26 @@ test_walk(const char *filename, int nodes) {
ISC_RUN_TEST_IMPL(walk) {
UNUSED(state);
test_walk(TESTS_DIR "/testdata/dbiterator/zone1.data", 13);
test_walk(TESTS_DIR "/testdata/dbiterator/zone1.data", 0, 12);
test_walk(TESTS_DIR "/testdata/dbiterator/zone1.data", DNS_DB_NONSEC3,
12);
test_walk(TESTS_DIR "/testdata/dbiterator/zone1.data", DNS_DB_NSEC3ONLY,
0);
}
ISC_RUN_TEST_IMPL(walk_nsec3) {
UNUSED(state);
test_walk(TESTS_DIR "/testdata/dbiterator/zone2.data", 33);
test_walk(TESTS_DIR "/testdata/dbiterator/zone2.data", 0, 32);
test_walk(TESTS_DIR "/testdata/dbiterator/zone2.data", DNS_DB_NONSEC3,
12);
test_walk(TESTS_DIR "/testdata/dbiterator/zone2.data", DNS_DB_NSEC3ONLY,
20);
}
/* reverse: walk database backwards */
static void
test_reverse(const char *filename) {
test_reverse(const char *filename, int flags, int nodes) {
isc_result_t result;
dns_db_t *db = NULL;
dns_dbiterator_t *iter = NULL;
@@ -137,7 +145,7 @@ test_reverse(const char *filename) {
result = dns_test_loaddb(&db, dns_dbtype_zone, TEST_ORIGIN, filename);
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_db_createiterator(db, 0, &iter);
result = dns_db_createiterator(db, flags, &iter);
assert_int_equal(result, ISC_R_SUCCESS);
for (result = dns_dbiterator_last(iter); result == ISC_R_SUCCESS;
@@ -152,7 +160,7 @@ test_reverse(const char *filename) {
i++;
}
assert_int_equal(i, 12);
assert_int_equal(i, nodes);
dns_dbiterator_destroy(&iter);
dns_db_detach(&db);
@@ -161,18 +169,26 @@ test_reverse(const char *filename) {
ISC_RUN_TEST_IMPL(reverse) {
UNUSED(state);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone1.data");
test_reverse(TESTS_DIR "/testdata/dbiterator/zone1.data", 0, 12);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone1.data",
DNS_DB_NONSEC3, 12);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone1.data",
DNS_DB_NSEC3ONLY, 0);
}
ISC_RUN_TEST_IMPL(reverse_nsec3) {
UNUSED(state);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone2.data");
test_reverse(TESTS_DIR "/testdata/dbiterator/zone2.data", 0, 32);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone2.data",
DNS_DB_NONSEC3, 12);
test_reverse(TESTS_DIR "/testdata/dbiterator/zone2.data",
DNS_DB_NSEC3ONLY, 20);
}
/* seek: walk database starting at a particular node */
static void
test_seek_node(const char *filename, int nodes) {
test_seek_node(const char *filename, int flags, int nodes) {
isc_result_t result;
dns_db_t *db = NULL;
dns_dbiterator_t *iter = NULL;
@@ -187,14 +203,19 @@ test_seek_node(const char *filename, int nodes) {
result = dns_test_loaddb(&db, dns_dbtype_zone, TEST_ORIGIN, filename);
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_db_createiterator(db, 0, &iter);
result = dns_db_createiterator(db, flags, &iter);
assert_int_equal(result, ISC_R_SUCCESS);
result = make_name("c." TEST_ORIGIN, seekname);
assert_int_equal(result, ISC_R_SUCCESS);
result = dns_dbiterator_seek(iter, seekname);
assert_int_equal(result, ISC_R_SUCCESS);
if (flags == DNS_DB_NSEC3ONLY) {
/* "c" isn't in the NSEC3 tree but the origin node is */
assert_int_equal(result, DNS_R_PARTIALMATCH);
} else {
assert_int_equal(result, ISC_R_SUCCESS);
}
while (result == ISC_R_SUCCESS) {
result = dns_dbiterator_current(iter, &node, name);
@@ -209,6 +230,31 @@ test_seek_node(const char *filename, int nodes) {
assert_int_equal(i, nodes);
/* now reset the iterator and walk backwards */
i = 0;
result = dns_dbiterator_seek(iter, seekname);
if (flags == DNS_DB_NSEC3ONLY) {
/* "c" isn't in the NSEC3 tree but the origin node is */
assert_int_equal(result, DNS_R_PARTIALMATCH);
nodes = 0;
} else {
assert_int_equal(result, ISC_R_SUCCESS);
nodes = 4;
}
while (result == ISC_R_SUCCESS) {
result = dns_dbiterator_current(iter, &node, name);
if (result == DNS_R_NEWORIGIN) {
result = ISC_R_SUCCESS;
}
assert_int_equal(result, ISC_R_SUCCESS);
dns_db_detachnode(db, &node);
result = dns_dbiterator_prev(iter);
i++;
}
assert_int_equal(i, nodes);
dns_dbiterator_destroy(&iter);
dns_db_detach(&db);
}
@@ -216,13 +262,21 @@ test_seek_node(const char *filename, int nodes) {
ISC_RUN_TEST_IMPL(seek_node) {
UNUSED(state);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone1.data", 10);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone1.data", 0, 9);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone1.data",
DNS_DB_NONSEC3, 9);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone1.data",
DNS_DB_NSEC3ONLY, 0);
}
ISC_RUN_TEST_IMPL(seek_node_nsec3) {
UNUSED(state);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone2.data", 30);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone2.data", 0, 29);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone2.data",
DNS_DB_NONSEC3, 9);
test_seek_node(TESTS_DIR "/testdata/dbiterator/zone2.data",
DNS_DB_NSEC3ONLY, 0);
}
/*