From 78886d4bedeef3dcffa1f9eebe431f35a03f3593 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Thu, 26 Mar 2020 10:57:38 +0100 Subject: [PATCH] Fix the statistic counter underflow in ns_client_t In case of normal fetch, the .recursionquota is attached and ns_statscounter_recursclients is incremented when the fetch is created. Then the .recursionquota is detached and the counter decremented in the fetch_callback(). In case of prefetch or rpzfetch, the quota is attached, but the counter is not incremented. When we reach the soft-quota, the function returns early but don't detach from the quota, and it gets destroyed during the ns_client_endrequest(), so no memory was leaked. But because the ns_statscounter_recursclients is only incremented during the normal fetch the counter would be incorrectly decremented on two occassions: 1) When we reached the softquota, because the quota was not properly detached 2) When the prefetch or rpzfetch was cancelled mid-flight and the callback function was never called. --- lib/ns/client.c | 6 ++++-- lib/ns/query.c | 13 +++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lib/ns/client.c b/lib/ns/client.c index 9900866c35..04a8da7d3e 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -235,8 +235,10 @@ ns_client_endrequest(ns_client_t *client) { */ if (client->recursionquota != NULL) { isc_quota_detach(&client->recursionquota); - ns_stats_decrement(client->sctx->nsstats, - ns_statscounter_recursclients); + if (client->query.prefetch == NULL) { + ns_stats_decrement(client->sctx->nsstats, + ns_statscounter_recursclients); + } } /* diff --git a/lib/ns/query.c b/lib/ns/query.c index a503109db0..46f1b59c89 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -2463,6 +2463,13 @@ prefetch_done(isc_task_t *task, isc_event_t *event) { } UNLOCK(&client->query.fetchlock); + /* + * We're done prefetching, detach from quota. + */ + if (client->recursionquota != NULL) { + isc_quota_detach(&client->recursionquota); + } + free_devent(client, &event, &devent); isc_nmhandle_unref(client->handle); } @@ -2488,6 +2495,9 @@ query_prefetch(ns_client_t *client, dns_name_t *qname, if (client->recursionquota == NULL) { result = isc_quota_attach(&client->sctx->recursionquota, &client->recursionquota); + if (result == ISC_R_SOFTQUOTA) { + isc_quota_detach(&client->recursionquota); + } if (result != ISC_R_SUCCESS) { return; } @@ -2698,6 +2708,9 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) { if (client->recursionquota == NULL) { result = isc_quota_attach(&client->sctx->recursionquota, &client->recursionquota); + if (result == ISC_R_SOFTQUOTA) { + isc_quota_detach(&client->recursionquota); + } if (result != ISC_R_SUCCESS) { return; }