In dns_qpiter_{prev,next}, defer dereference_iter_node call

dns_qpiter_{prev,next} requires the current iterator node to still be
valid which might not always the case after dereference_iter_node was
called.  Currently, this is ensured via closeversion() mechanism, but it
is not guaranteed to be true in the future.

Move the call to dereference_iter_node to after the dns_qpiter_prev()
and dns_qpiter_next() to prevent a possible use-after-free of the
current iterator node.

(cherry picked from commit 9914bd383e)
This commit is contained in:
Ondřej Surý 2025-12-05 12:29:32 +01:00
parent b23a364be9
commit 89478d95c3
No known key found for this signature in database
GPG key ID: 2820F37E873DEA41

View file

@ -4384,6 +4384,9 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
isc_result_t result;
qpdb_dbiterator_t *qpdbiter = (qpdb_dbiterator_t *)iterator;
qpzonedb_t *qpdb = (qpzonedb_t *)iterator->db;
qpznode_t *node = NULL;
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
isc_rwlock_t *nlock = NULL;
REQUIRE(qpdbiter->node != NULL);
@ -4391,7 +4394,12 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
return qpdbiter->result;
}
dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
/*
* Defer the release of the current node until we have the prev node
* from the QP tree.
*/
node = qpdbiter->node;
qpdbiter->node = NULL;
result = dns_qpiter_prev(qpdbiter->current, NULL,
(void **)&qpdbiter->node, NULL);
@ -4417,6 +4425,14 @@ dbiterator_prev(dns_dbiterator_t *iterator DNS__DB_FLARG) {
}
}
/*
* We have the prev node, we can release the previous current.
*/
nlock = &qpdb->buckets[node->locknum].lock;
NODE_RDLOCK(nlock, &nlocktype);
qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
NODE_UNLOCK(nlock, &nlocktype);
if (result == ISC_R_SUCCESS) {
reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
} else {
@ -4432,6 +4448,9 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
isc_result_t result;
qpdb_dbiterator_t *qpdbiter = (qpdb_dbiterator_t *)iterator;
qpzonedb_t *qpdb = (qpzonedb_t *)iterator->db;
qpznode_t *node = NULL;
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
isc_rwlock_t *nlock = NULL;
REQUIRE(qpdbiter->node != NULL);
@ -4439,7 +4458,12 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
return qpdbiter->result;
}
dereference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
/*
* Defer the release of the current node until we have the next node
* from the QP tree.
*/
node = qpdbiter->node;
qpdbiter->node = NULL;
result = dns_qpiter_next(qpdbiter->current, NULL,
(void **)&qpdbiter->node, NULL);
@ -4476,6 +4500,14 @@ dbiterator_next(dns_dbiterator_t *iterator DNS__DB_FLARG) {
}
}
/*
* We have the next node, we can release the previous current.
*/
nlock = &qpdb->buckets[node->locknum].lock;
NODE_RDLOCK(nlock, &nlocktype);
qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
NODE_UNLOCK(nlock, &nlocktype);
if (result == ISC_R_SUCCESS) {
reference_iter_node(qpdbiter DNS__DB_FLARG_PASS);
} else {