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 89478d95c39768793fa17dffaa2ca02cbd75f643)

diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c
index 2c59fb7f98c4445086d476b5f0ec91e251921c85..9b36982348e9b43d46311fe166058da7c64c23ca 100644 (file)
--- a/lib/dns/qpzone.c
+++ b/lib/dns/qpzone.c
@@ -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 {