From fb7bbbd1be20632db28a928f49c4082373358b64 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 27 Sep 2023 11:22:43 +0000 Subject: [PATCH 1/2] Don't use an uninitialized link on an error path Move the block on the error path, where the link is checked, to a place where it makes sense, to avoid accessing an unitialized link when jumping to the 'cleanup_query' label from 4 different places. The link is initialized only after those jumps happen. In addition, initilize the link when creating the object, to avoid similar errors. --- lib/dns/resolver.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index aa19fd5b09..bd467d5269 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1991,9 +1991,12 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, INSIST(ISC_LIST_EMPTY(fctx->validators)); query = isc_mem_get(fctx->mctx, sizeof(*query)); - *query = (resquery_t){ .options = options, - .addrinfo = addrinfo, - .dispatchmgr = res->view->dispatchmgr }; + *query = (resquery_t){ + .options = options, + .addrinfo = addrinfo, + .dispatchmgr = res->view->dispatchmgr, + .link = ISC_LINK_INITIALIZER, + }; #if DNS_RESOLVER_TRACE fprintf(stderr, "rctx_init:%s:%s:%d:%p->references = 1\n", __func__, @@ -2141,7 +2144,6 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, LOCK(&fctx->lock); INSIST(!SHUTTINGDOWN(fctx)); fetchctx_attach(fctx, &query->fctx); - ISC_LINK_INIT(query, link); query->magic = QUERY_MAGIC; if ((query->options & DNS_FETCHOPT_TCP) == 0) { @@ -2186,6 +2188,13 @@ cleanup_udpfetch: } } + LOCK(&fctx->lock); + if (ISC_LINK_LINKED(query, link)) { + atomic_fetch_sub_release(&fctx->nqueries, 1); + ISC_LIST_UNLINK(fctx->queries, query, link); + } + UNLOCK(&fctx->lock); + cleanup_dispatch: fetchctx_detach(&query->fctx); @@ -2194,13 +2203,6 @@ cleanup_dispatch: } cleanup_query: - LOCK(&fctx->lock); - if (ISC_LINK_LINKED(query, link)) { - atomic_fetch_sub_release(&fctx->nqueries, 1); - ISC_LIST_UNLINK(fctx->queries, query, link); - } - UNLOCK(&fctx->lock); - query->magic = 0; dns_message_detach(&query->rmessage); isc_mem_put(fctx->mctx, query, sizeof(*query)); From 9c545c3513a1f78dc21e83a823041da790925158 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 27 Sep 2023 11:34:30 +0000 Subject: [PATCH 2/2] Add a CHANGES note for [GL #4331] --- CHANGES | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES b/CHANGES index 75dea389ee..362f7bc2a7 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,7 @@ +6261. [bug] Fix a possible assertion failure on an error path in + resolver.c:fctx_query(), when using an uninitialized + link. [GL #4331] + 6260. [func] Added opptions to the QP trie that will be needed when it is used as a zone or cache database: backward iteration, and retrieval of DNSSEC predecessor