diff --git a/CHANGES b/CHANGES index db66391e2a..1c450972ee 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +5998. [bug] The RecursClients statistics counter could overflow + in certain resolution scenarios. [GL #3584] + 5997. [cleanup] Less ceremonial UNEXPECTED_ERROR() and FATAL_ERROR() reporting macros. [GL !6914] diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 79b66041c7..f7de4ebfd7 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -40,4 +40,5 @@ Feature Changes Bug Fixes ~~~~~~~~~ -- None. +- The RecursClients statistics counter could overflow in certain resolution + scenarios. This has been fixed. :gl:`#3584` diff --git a/lib/ns/client.c b/lib/ns/client.c index 47af2c717b..2b3da21b21 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -269,10 +269,8 @@ ns_client_endrequest(ns_client_t *client) { */ if (client->recursionquota != NULL) { isc_quota_detach(&client->recursionquota); - if (client->query.prefetch == NULL) { - ns_stats_decrement(client->sctx->nsstats, - ns_statscounter_recursclients); - } + ns_stats_decrement(client->sctx->nsstats, + ns_statscounter_recursclients); } /* diff --git a/lib/ns/query.c b/lib/ns/query.c index afc2e5ea81..21ad1a34ce 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2543,11 +2543,10 @@ prefetch_done(isc_task_t *task, isc_event_t *event) { */ if (client->recursionquota != NULL) { isc_quota_detach(&client->recursionquota); + ns_stats_decrement(client->sctx->nsstats, + ns_statscounter_recursclients); } - ns_stats_decrement(client->sctx->nsstats, - ns_statscounter_recursclients); - free_devent(client, &event, &devent); isc_nmhandle_detach(&client->prefetchhandle); } @@ -2575,6 +2574,8 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, &client->recursionquota); switch (result) { case ISC_R_SUCCESS: + ns_stats_increment(client->sctx->nsstats, + ns_statscounter_recursclients); break; case ISC_R_SOFTQUOTA: isc_quota_detach(&client->recursionquota); @@ -2584,9 +2585,6 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, } } - ns_stats_increment(client->sctx->nsstats, - ns_statscounter_recursclients); - tmprdataset = ns_client_newrdataset(client); if (tmprdataset == NULL) { return; @@ -2793,6 +2791,8 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { &client->recursionquota); switch (result) { case ISC_R_SUCCESS: + ns_stats_increment(client->sctx->nsstats, + ns_statscounter_recursclients); break; case ISC_R_SOFTQUOTA: isc_quota_detach(&client->recursionquota); @@ -2802,9 +2802,6 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { } } - ns_stats_increment(client->sctx->nsstats, - ns_statscounter_recursclients); - tmprdataset = ns_client_newrdataset(client); if (tmprdataset == NULL) { return; @@ -6252,11 +6249,10 @@ fetch_callback(isc_task_t *task, isc_event_t *event) { if (client->recursionquota != NULL) { isc_quota_detach(&client->recursionquota); + ns_stats_decrement(client->sctx->nsstats, + ns_statscounter_recursclients); } - ns_stats_decrement(client->sctx->nsstats, - ns_statscounter_recursclients); - LOCK(&client->manager->reclock); if (ISC_LINK_LINKED(client, rlink)) { ISC_LIST_UNLINK(client->manager->recursing, client, rlink); @@ -6390,6 +6386,11 @@ check_recursionquota(ns_client_t *client) { if (client->recursionquota == NULL) { result = isc_quota_attach(&client->sctx->recursionquota, &client->recursionquota); + if (result == ISC_R_SUCCESS || result == ISC_R_SOFTQUOTA) { + ns_stats_increment(client->sctx->nsstats, + ns_statscounter_recursclients); + } + if (result == ISC_R_SOFTQUOTA) { isc_stdtime_t now; isc_stdtime_get(&now); @@ -6472,9 +6473,6 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname, return (result); } - ns_stats_increment(client->sctx->nsstats, - ns_statscounter_recursclients); - /* * Invoke the resolver. */ @@ -6765,11 +6763,10 @@ query_hookresume(isc_task_t *task, isc_event_t *event) { if (client->recursionquota != NULL) { isc_quota_detach(&client->recursionquota); + ns_stats_decrement(client->sctx->nsstats, + ns_statscounter_recursclients); } - ns_stats_decrement(client->sctx->nsstats, - ns_statscounter_recursclients); - LOCK(&client->manager->reclock); if (ISC_LINK_LINKED(client, rlink)) { ISC_LIST_UNLINK(client->manager->recursing, client, rlink); @@ -6905,9 +6902,6 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync, goto cleanup; } - ns_stats_increment(client->sctx->nsstats, - ns_statscounter_recursclients); - saved_qctx = isc_mem_get(client->mctx, sizeof(*saved_qctx)); qctx_save(qctx, saved_qctx); result = runasync(saved_qctx, client->mctx, arg, client->task,