mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-09-01 06:55:30 +00:00
Code Issues Releases Wiki Activity

Merge branch 'each-broken-qpiter' into 'main'

Browse Source 
more fix_iterator() bugs

See merge request isc-projects/bind9!8606
This commit is contained in:
Evan Hunt
2023-12-21 17:57:47 +00:00
parent 93cc89e4d3 ea9a8cb392
commit e997a738d6
2 changed files with 157 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
lib/dns/qp.c
View File

@@ -2029,13 +2029,14 @@ add_link(dns_qpchain_t *chain, dns_qpnode_t *node, size_t offset) {
INSIST(chain->len <= DNS_NAME_MAXLABELS); INSIST(chain->len <= DNS_NAME_MAXLABELS);
} }
static inline void static inline dns_qpnode_t *
prevleaf(dns_qpiter_t *it) { prevleaf(dns_qpiter_t *it) {
isc_result_t result = dns_qpiter_prev(it, NULL, NULL, NULL); isc_result_t result = dns_qpiter_prev(it, NULL, NULL, NULL);
if (result == ISC_R_NOMORE) { if (result == ISC_R_NOMORE) {
result = dns_qpiter_prev(it, NULL, NULL, NULL); result = dns_qpiter_prev(it, NULL, NULL, NULL);
} }
RUNTIME_CHECK(result == ISC_R_SUCCESS); RUNTIME_CHECK(result == ISC_R_SUCCESS);
return (it->stack[it->sp]);
} }
static inline dns_qpnode_t * static inline dns_qpnode_t *
@@ -2046,6 +2047,7 @@ greatest_leaf(dns_qpreadable_t qpr, dns_qpnode_t *n, dns_qpiter_t *iter) {
iter->stack[++iter->sp] = n; iter->stack[++iter->sp] = n;
n = ref_ptr(qpr, ref); n = ref_ptr(qpr, ref);
} }
iter->stack[++iter->sp] = n;
return (n); return (n);
} }
@@ -2113,6 +2115,17 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
return (leaf); return (leaf);
} }
/*
* Special case: if the search key differs even before the root
* key offset, it means the name desired either precedes or
* follows the entire range of names in the database, and
* popping up the stack won't help us, so just move the
* iterator one step back from the origin and return.
*/
if (to < branch_key_offset(iter->stack[0])) {
dns_qpiter_init(qp, iter);
return (prevleaf(iter));
}
/* /*
* As long as the branch offset point is after the point where the * As long as the branch offset point is after the point where the
* search key differs, we need to branch up and find a better leaf * search key differs, we need to branch up and find a better leaf
@@ -2126,12 +2139,11 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
* go to the parent branch and iterate back to the * go to the parent branch and iterate back to the
* predecessor from that point. * predecessor from that point.
*/ */
iter->sp--; n = prevleaf(iter);
prevleaf(iter);
n = iter->stack[iter->sp];
leaf = n; leaf = n;
} else { } else {
if (is_branch(n)) { if (is_branch(n)) {
iter->sp--;
n = greatest_leaf(qp, n, iter); n = greatest_leaf(qp, n, iter);
} }
return (n); return (n);
@@ -2155,8 +2167,7 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
* wanted; use the iterator to walk back to the * wanted; use the iterator to walk back to the
* predecessor. * predecessor.
*/ */
prevleaf(iter); n = prevleaf(iter);
n = iter->stack[iter->sp--];
} else { } else {
/* /*
* The name we want would've been after some twig in * The name we want would've been after some twig in
@@ -2177,7 +2188,7 @@ fix_iterator(dns_qpreader_t *qp, dns_qpiter_t *iter, dns_qpnode_t *start,
*/ */
if (to <= searchlen && to <= foundlen && search[to] < found[to]) if (to <= searchlen && to <= foundlen && search[to] < found[to])
{ {
prevleaf(iter); n = prevleaf(iter);
} }
} }
@@ -2272,6 +2283,8 @@ dns_qp_lookup(dns_qpreadable_t qpr, const dns_name_t *name,
*/ */
n = fix_iterator(qp, iter, n, search, searchlen, bit, n = fix_iterator(qp, iter, n, search, searchlen, bit,
offset); offset);
iter->stack[iter->sp] = NULL;
iter->sp--;
} else { } else {
/* /*
* this branch is a dead end, and the predecessor * this branch is a dead end, and the predecessor

209
tests/dns/qp_test.c
View File

@@ -581,6 +581,7 @@ struct check_predecessors {
const char *query; const char *query;
const char *predecessor; const char *predecessor;
isc_result_t result; isc_result_t result;
int remaining;
}; };
static void static void
@@ -589,11 +590,12 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
dns_fixedname_t fn1, fn2; dns_fixedname_t fn1, fn2;
dns_name_t *name = dns_fixedname_initname(&fn1); dns_name_t *name = dns_fixedname_initname(&fn1);
dns_name_t *pred = dns_fixedname_initname(&fn2); dns_name_t *pred = dns_fixedname_initname(&fn2);
char *namestr = NULL;
for (int i = 0; check[i].query != NULL; i++) { for (int i = 0; check[i].query != NULL; i++) {
dns_qpiter_t it; dns_qpiter_t it;
char *predname = NULL;
namestr = NULL;
dns_test_namefromstring(check[i].query, &fn1); dns_test_namefromstring(check[i].query, &fn1);
result = dns_qp_lookup(qp, name, NULL, &it, NULL, NULL, NULL); result = dns_qp_lookup(qp, name, NULL, &it, NULL, NULL, NULL);
#if 0 #if 0
@@ -621,15 +623,35 @@ check_predecessors(dns_qp_t *qp, struct check_predecessors check[]) {
} }
assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(result, ISC_R_SUCCESS);
result = dns_name_tostring(pred, &predname, mctx); result = dns_name_tostring(pred, &namestr, mctx);
#if 0 #if 0
fprintf(stderr, "... expected predecessor %s got %s\n", fprintf(stderr, "... expected predecessor %s got %s\n",
check[i].predecessor, predname); check[i].predecessor, namestr);
#endif #endif
assert_int_equal(result, ISC_R_SUCCESS); assert_int_equal(result, ISC_R_SUCCESS);
assert_string_equal(namestr, check[i].predecessor);
assert_string_equal(predname, check[i].predecessor); #if 0
isc_mem_free(mctx, predname); fprintf(stderr, "%d: remaining names after %s:\n", i, namestr);
#endif
isc_mem_free(mctx, namestr);
int j = 0;
while (dns_qpiter_next(&it, name, NULL, NULL) == ISC_R_SUCCESS)
{
#if 0
result = dns_name_tostring(name, &namestr, mctx);
assert_int_equal(result, ISC_R_SUCCESS);
fprintf(stderr, "%s%s", j > 0 ? "->" : "", namestr);
isc_mem_free(mctx, namestr);
#endif
j++;
}
#if 0
fprintf(stderr, "\n...expected %d got %d\n",
check[i].remaining, j);
#endif
assert_int_equal(j, check[i].remaining);
} }
} }
@@ -650,40 +672,40 @@ ISC_RUN_TEST_IMPL(predecessors) {
/* first check: no root label in the database */ /* first check: no root label in the database */
static struct check_predecessors check1[] = { static struct check_predecessors check1[] = {
{ ".", "moops.", ISC_R_NOTFOUND }, { ".", "moops.", ISC_R_NOTFOUND, 0 },
{ "a.", "moops.", ISC_R_SUCCESS }, { "a.", "moops.", ISC_R_SUCCESS, 0 },
{ "b.a.", "a.", ISC_R_SUCCESS }, { "b.a.", "a.", ISC_R_SUCCESS, 14 },
{ "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, { "b.", "e.d.c.b.a.", ISC_R_SUCCESS, 11 },
{ "aaa.a.", "a.", DNS_R_PARTIALMATCH }, { "aaa.a.", "a.", DNS_R_PARTIALMATCH, 14 },
{ "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH, 11 },
{ "d.c.", "c.b.b.", ISC_R_NOTFOUND }, { "d.c.", "c.b.b.", ISC_R_NOTFOUND, 9 },
{ "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH, 12 },
{ "a.b.c.e.f.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "a.b.c.e.f.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "z.y.x.", "moops.", ISC_R_NOTFOUND }, { "z.y.x.", "moops.", ISC_R_NOTFOUND, 0 },
{ "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH, 6 },
{ "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH, 6 },
{ "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH, 7 },
{ "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH, 11 },
{ "0.b.c.d.e.", "x.k.c.d.", ISC_R_NOTFOUND }, { "0.b.c.d.e.", "x.k.c.d.", ISC_R_NOTFOUND, 6 },
{ "b.d.", "c.b.b.", ISC_R_NOTFOUND }, { "b.d.", "c.b.b.", ISC_R_NOTFOUND, 9 },
{ "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "moor.", "moops.", ISC_R_NOTFOUND }, { "moor.", "moops.", ISC_R_NOTFOUND, 0 },
{ "mopbop.", "moops.", ISC_R_NOTFOUND }, { "mopbop.", "moops.", ISC_R_NOTFOUND, 0 },
{ "moppop.", "moops.", ISC_R_NOTFOUND }, { "moppop.", "moops.", ISC_R_NOTFOUND, 0 },
{ "mopps.", "moops.", ISC_R_NOTFOUND }, { "mopps.", "moops.", ISC_R_NOTFOUND, 0 },
{ "mopzop.", "moops.", ISC_R_NOTFOUND }, { "mopzop.", "moops.", ISC_R_NOTFOUND, 0 },
{ "mop.", "moops.", ISC_R_NOTFOUND }, { "mop.", "moops.", ISC_R_NOTFOUND, 0 },
{ "monbop.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monbop.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "monpop.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monpop.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "monps.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monps.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "monzop.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monzop.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "mon.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "moop.", "moon.", ISC_R_NOTFOUND }, { "moop.", "moon.", ISC_R_NOTFOUND, 1 },
{ "moopser.", "moops.", ISC_R_NOTFOUND }, { "moopser.", "moops.", ISC_R_NOTFOUND, 0 },
{ "monky.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monky.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "monkey.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monkey.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ "monker.", "a.b.c.d.e.", ISC_R_NOTFOUND }, { "monker.", "a.b.c.d.e.", ISC_R_NOTFOUND, 5 },
{ NULL, NULL, 0 } { NULL, NULL, 0, 0 }
}; };
check_predecessors(qp, check1); check_predecessors(qp, check1);
@@ -693,39 +715,39 @@ ISC_RUN_TEST_IMPL(predecessors) {
insert_str(qp, root); insert_str(qp, root);
static struct check_predecessors check2[] = { static struct check_predecessors check2[] = {
{ ".", "moops.", ISC_R_SUCCESS }, { ".", "moops.", ISC_R_SUCCESS, 0 },
{ "a.", ".", ISC_R_SUCCESS }, { "a.", ".", ISC_R_SUCCESS, 15 },
{ "b.a.", "a.", ISC_R_SUCCESS }, { "b.a.", "a.", ISC_R_SUCCESS, 14 },
{ "b.", "e.d.c.b.a.", ISC_R_SUCCESS }, { "b.", "e.d.c.b.a.", ISC_R_SUCCESS, 11 },
{ "aaa.a.", "a.", DNS_R_PARTIALMATCH }, { "aaa.a.", "a.", DNS_R_PARTIALMATCH, 14 },
{ "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "ddd.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH, 11 },
{ "d.c.", "c.b.b.", DNS_R_PARTIALMATCH }, { "d.c.", "c.b.b.", DNS_R_PARTIALMATCH, 9 },
{ "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH }, { "1.2.c.b.a.", "c.b.a.", DNS_R_PARTIALMATCH, 12 },
{ "a.b.c.e.f.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "a.b.c.e.f.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "z.y.x.", "moops.", DNS_R_PARTIALMATCH }, { "z.y.x.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "w.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH, 6 },
{ "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "z.z.z.z.k.c.d.", "x.k.c.d.", DNS_R_PARTIALMATCH, 6 },
{ "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH }, { "w.k.c.d.", "a.b.c.d.", DNS_R_PARTIALMATCH, 7 },
{ "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH }, { "d.a.", "e.d.c.b.a.", DNS_R_PARTIALMATCH, 11 },
{ "0.b.c.d.e.", "x.k.c.d.", DNS_R_PARTIALMATCH }, { "0.b.c.d.e.", "x.k.c.d.", DNS_R_PARTIALMATCH, 6 },
{ "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "moor.", "moops.", DNS_R_PARTIALMATCH }, { "moor.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "mopbop.", "moops.", DNS_R_PARTIALMATCH }, { "mopbop.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "moppop.", "moops.", DNS_R_PARTIALMATCH }, { "moppop.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "mopps.", "moops.", DNS_R_PARTIALMATCH }, { "mopps.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "mopzop.", "moops.", DNS_R_PARTIALMATCH }, { "mopzop.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "mop.", "moops.", DNS_R_PARTIALMATCH }, { "mop.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "monbop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monbop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "monpop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monpop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "monps.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monps.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "monzop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monzop.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "mon.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "moop.", "moon.", DNS_R_PARTIALMATCH }, { "moop.", "moon.", DNS_R_PARTIALMATCH, 1 },
{ "moopser.", "moops.", DNS_R_PARTIALMATCH }, { "moopser.", "moops.", DNS_R_PARTIALMATCH, 0 },
{ "monky.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monky.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "monkey.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monkey.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ "monker.", "a.b.c.d.e.", DNS_R_PARTIALMATCH }, { "monker.", "a.b.c.d.e.", DNS_R_PARTIALMATCH, 5 },
{ NULL, NULL, 0 } { NULL, NULL, 0, 0 }
}; };
check_predecessors(qp, check2); check_predecessors(qp, check2);
@@ -733,6 +755,48 @@ ISC_RUN_TEST_IMPL(predecessors) {
dns_qp_destroy(&qp); dns_qp_destroy(&qp);
} }
/*
* this is a regression test for an infinite loop that could
* previously occur in fix_iterator()
*/
ISC_RUN_TEST_IMPL(fixiterator) {
dns_qp_t *qp = NULL;
const char insert[][32] = { "dynamic.",
"a.dynamic.",
"aaaa.dynamic.",
"cdnskey.dynamic.",
"cds.dynamic.",
"cname.dynamic.",
"dname.dynamic.",
"dnskey.dynamic.",
"ds.dynamic.",
"mx.dynamic.",
"ns.dynamic.",
"nsec.dynamic.",
"private-cdnskey.dynamic.",
"private-dnskey.dynamic.",
"rrsig.dynamic.",
"txt.dynamic.",
"" };
int i = 0;
dns_qp_create(mctx, &string_methods, NULL, &qp);
while (insert[i][0] != '\0') {
insert_str(qp, insert[i++]);
}
static struct check_predecessors check1[] = {
{ "newtext.dynamic.", "mx.dynamic.", DNS_R_PARTIALMATCH, 6 },
{ "absent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
{ "nonexistent.", "txt.dynamic.", ISC_R_NOTFOUND, 0 },
{ NULL, NULL, 0, 0 }
};
check_predecessors(qp, check1);
dns_qp_destroy(&qp);
}
ISC_TEST_LIST_START ISC_TEST_LIST_START
ISC_TEST_ENTRY(qpkey_name) ISC_TEST_ENTRY(qpkey_name)
ISC_TEST_ENTRY(qpkey_sort) ISC_TEST_ENTRY(qpkey_sort)
@@ -740,6 +804,7 @@ ISC_TEST_ENTRY(qpiter)
ISC_TEST_ENTRY(partialmatch) ISC_TEST_ENTRY(partialmatch)
ISC_TEST_ENTRY(qpchain) ISC_TEST_ENTRY(qpchain)
ISC_TEST_ENTRY(predecessors) ISC_TEST_ENTRY(predecessors)
ISC_TEST_ENTRY(fixiterator)
ISC_TEST_LIST_END ISC_TEST_LIST_END
ISC_TEST_MAIN ISC_TEST_MAIN