Merge branch '4467-fix-stats-export-overflow' into 'main'

Avoid overflow during statistics dump

Closes #4467

See merge request isc-projects/bind9!8532

CHANGES

@@ -1,3 +1,7 @@
+6300.	[bug]		Fix statistics export to use full 64 bit signed numbers
+			instead of truncating values to unsigned 32 bits.
+			[GL #4467]
+
 6299.	[port]		NetBSD has added 'hmac' to libc which collides with our
 			use of 'hmac'. [GL #4478]
 

doc/notes/notes-current.rst

@@ -35,7 +35,9 @@ Feature Changes
 Bug Fixes
 ~~~~~~~~~
 
-- None.
+- Fix statistics export to use full 64 bit signed numbers instead of truncating
+  values to unsigned 32 bits. Export was truncating values since BIND 9.15.0.
+  :gl:`#4467`
 
 Known Issues
 ~~~~~~~~~~~~

lib/isc/include/isc/types.h

@@ -20,6 +20,7 @@
  * OS-specific types, from the OS-specific include directories.
  */
 #include <limits.h>
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -72,6 +73,7 @@ typedef ISC_LIST(isc_sockaddr_t) isc_sockaddrlist_t; /*%< Socket Address List
 						      * */
 typedef struct isc_stats      isc_stats_t;	     /*%< Statistics */
 typedef int_fast64_t	      isc_statscounter_t;
+typedef atomic_int_fast64_t   isc_atomic_statscounter_t;
 typedef struct isc_symtab     isc_symtab_t;	/*%< Symbol Table */
 typedef struct isc_textregion isc_textregion_t; /*%< Text Region */
 typedef struct isc_time	      isc_time_t;	/*%< Time */

lib/isc/stats.c

@@ -27,14 +27,22 @@
 #define ISC_STATS_MAGIC	   ISC_MAGIC('S', 't', 'a', 't')
 #define ISC_STATS_VALID(x) ISC_MAGIC_VALID(x, ISC_STATS_MAGIC)
 
-typedef atomic_int_fast64_t isc__atomic_statcounter_t;
+/*
+ * Statistics are counted with an atomic int_fast64_t but exported to functions
+ * taking uint64_t (isc_stats_dumper_t). A 128-bit native and fast architecture
+ * doesn't exist in reality so these two are the same thing in practise.
+ * However, a silent truncation happening silently in the future is still not
+ * acceptable.
+ */
+STATIC_ASSERT(sizeof(isc_statscounter_t) <= sizeof(uint64_t),
+	      "Exported statistics must fit into the statistic counter size");
 
 struct isc_stats {
 	unsigned int magic;
 	isc_mem_t *mctx;
 	isc_refcount_t references;
 	int ncounters;
-	isc__atomic_statcounter_t *counters;
+	isc_atomic_statscounter_t *counters;
 };
 
 void
@@ -58,7 +66,7 @@ isc_stats_detach(isc_stats_t **statsp) {
 	if (isc_refcount_decrement(&stats->references) == 1) {
 		isc_refcount_destroy(&stats->references);
 		isc_mem_cput(stats->mctx, stats->counters, stats->ncounters,
-			     sizeof(isc__atomic_statcounter_t));
+			     sizeof(isc_atomic_statscounter_t));
 		isc_mem_putanddetach(&stats->mctx, stats, sizeof(*stats));
 	}
 }
@@ -75,7 +83,7 @@ isc_stats_create(isc_mem_t *mctx, isc_stats_t **statsp, int ncounters) {
 	REQUIRE(statsp != NULL && *statsp == NULL);
 
 	isc_stats_t *stats = isc_mem_get(mctx, sizeof(*stats));
-	size_t counters_alloc_size = sizeof(isc__atomic_statcounter_t) *
+	size_t counters_alloc_size = sizeof(isc_atomic_statscounter_t) *
 				     ncounters;
 	stats->counters = isc_mem_get(mctx, counters_alloc_size);
 	isc_refcount_init(&stats->references, 1);
@@ -116,7 +124,8 @@ isc_stats_dump(isc_stats_t *stats, isc_stats_dumper_t dump_fn, void *arg,
 	REQUIRE(ISC_STATS_VALID(stats));
 
 	for (i = 0; i < stats->ncounters; i++) {
-		uint32_t counter = atomic_load_acquire(&stats->counters[i]);
+		isc_statscounter_t counter =
+			atomic_load_acquire(&stats->counters[i]);
 		if ((options & ISC_STATSDUMP_VERBOSE) == 0 && counter == 0) {
 			continue;
 		}
@@ -160,7 +169,7 @@ void
 isc_stats_resize(isc_stats_t **statsp, int ncounters) {
 	isc_stats_t *stats;
 	size_t counters_alloc_size;
-	isc__atomic_statcounter_t *newcounters;
+	isc_atomic_statscounter_t *newcounters;
 
 	REQUIRE(statsp != NULL && *statsp != NULL);
 	REQUIRE(ISC_STATS_VALID(*statsp));
@@ -173,7 +182,7 @@ isc_stats_resize(isc_stats_t **statsp, int ncounters) {
 	}
 
 	/* Grow number of counters. */
-	counters_alloc_size = sizeof(isc__atomic_statcounter_t) * ncounters;
+	counters_alloc_size = sizeof(isc_atomic_statscounter_t) * ncounters;
 	newcounters = isc_mem_get(stats->mctx, counters_alloc_size);
 	for (int i = 0; i < ncounters; i++) {
 		atomic_init(&newcounters[i], 0);
@@ -183,7 +192,7 @@ isc_stats_resize(isc_stats_t **statsp, int ncounters) {
 		atomic_store_release(&newcounters[i], counter);
 	}
 	isc_mem_cput(stats->mctx, stats->counters, stats->ncounters,
-		     sizeof(isc__atomic_statcounter_t));
+		     sizeof(isc_atomic_statscounter_t));
 	stats->counters = newcounters;
 	stats->ncounters = ncounters;
 }