From 03183baa6dc0273b1ee40bab16f6e40889263520 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Wed, 25 Oct 2023 14:59:55 -0700 Subject: [PATCH] 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. --- lib/dns/qp.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/lib/dns/qp.c b/lib/dns/qp.c index 8d94af9c08..24be5ce18f 100644 --- 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(); } /***********************************************************************