From: Ondřej Surý <ondrej@sury.org>
Date: Thu, 26 Mar 2020 09:57:38 +0000 (+0100)
Subject: Fix the statistic counter underflow in ns_client_t
X-Git-Tag: v9.17.1~12^2~1
X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=78886d4bedeef3dcffa1f9eebe431f35a03f3593;p=thirdparty%2Fbind9.git

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.
---

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 9900866c356..04a8da7d3e9 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 a503109db0f..46f1b59c892 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;
 		}