Prevent a possible race in dns_qpmulti_query() and _snapshot()

The `.reader` member of dns_qpmulti_t was accessed without RCU
protection; reader_open() calls rcu_dereference() on it, and this
call needs to be inside an RCU critical section.

A similar problem was identified in the dns_qpmulti_snapshot() - the
RCU critical section was completely missing.

These are relicts of the isc_qsbr - in the QSBR mode the rcu_read_lock()
and rcu_read_unlock() are no-ops and whole event loop is a critical section.

diff --git a/lib/dns/qp.c b/lib/dns/qp.c
index 8d94af9c08e0a1c38f5bc8543e15aa817a376f1a..24be5ce18fd28fe9d51c198d58544c37235809b8 100644 (file)
--- a/lib/dns/qp.c
+++ b/lib/dns/qp.c
@@ -1333,11 +1333,11 @@ dns_qpmulti_query(dns_qpmulti_t *multi, dns_qpread_t *qp) {
        REQUIRE(QPMULTI_VALID(multi));
        REQUIRE(qp != NULL);
 
-       dns_qpmulti_t *whence = reader_open(multi, qp);
-       INSIST(whence == multi);
-
        qp->tid = isc_tid();
        rcu_read_lock();
+
+       dns_qpmulti_t *whence = reader_open(multi, qp);
+       INSIST(whence == multi);
 }
 
 void
@@ -1358,12 +1358,15 @@ dns_qpmulti_snapshot(dns_qpmulti_t *multi, dns_qpsnap_t **qpsp) {
        REQUIRE(QPMULTI_VALID(multi));
        REQUIRE(qpsp != NULL && *qpsp == NULL);
 
+       rcu_read_lock();
+
        LOCK(&multi->mutex);
 
        dns_qp_t *qpw = &multi->writer;
        size_t bytes = sizeof(dns_qpsnap_t) + sizeof(dns_qpbase_t) +
                       sizeof(qpw->base->ptr[0]) * qpw->chunk_max;
        dns_qpsnap_t *qps = isc_mem_allocate(qpw->mctx, bytes);
+
        qps->whence = reader_open(multi, qps);
        INSIST(qps->whence == multi);
 
@@ -1414,6 +1417,8 @@ dns_qpsnap_destroy(dns_qpmulti_t *multi, dns_qpsnap_t **qpsp) {
 
        *qpsp = NULL;
        UNLOCK(&multi->mutex);
+
+       rcu_read_unlock();
 }
 
 /***********************************************************************