CLEANUP: counters: only retrieve zeroes for unallocated extra_counters

Since version 2.4 with commit 7f8f6cb926 ("BUG/MEDIUM: stats: prevent
crash if counters not alloc with dummy one") we can afford to always
update extra_counters because we know they're always either allocated
or linked to a dedicated trash. However, the ->fill_stats() callbacks
continue to access such values, making it technically possible to
retrieve random counters from this trash, which is not really clean.
Let's implement an explicit test in the ->fill_stats() functions to
only return 0 for the metric when not allocated like this. It's much
cleaner because it guarantees that we're returning an empty counter
in this case rather than random values.

The situation currently happens for dummy servers like the ones used
in Lua proxies as well as those used by rings (e.g. used for logging
or traces). Normally, none of the objects retrieved via stats or
Prometheus is concerned by this unallocated extra_counters situation,
so this is more about a cleanup than a real fix.

diff --git a/src/h3_stats.c b/src/h3_stats.c
index c89ce584e38fa68038033557051dbd9f8b91dd35..f82c7042283561f19d5bb5204abaefcd2b17d57c 100644 (file)
--- a/src/h3_stats.c
+++ b/src/h3_stats.c
@@ -132,11 +132,16 @@ static struct h3_counters {
 static int h3_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                          struct field *stats, unsigned int *selected_field)
 {
-       struct h3_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < H3_STATS_COUNT; current_field++) {
                struct field metric = { 0 };
+               struct h3_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                /* h3 frame type counters */
@@ -233,6 +238,7 @@ static int h3_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;

diff --git a/src/mux_h1.c b/src/mux_h1.c
index fc81636adf2078faf718526943523187787b6643..b37d2b6927a94887fa43aba291313d40d6b386f3 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -275,11 +275,16 @@ static struct h1_counters {
 static int h1_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                          struct field *stats, unsigned int *selected_field)
 {
-       struct h1_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < H1_STATS_COUNT; current_field++) {
                struct field metric = { 0 };
+               struct h1_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                case H1_ST_OPEN_CONN:
@@ -316,6 +321,7 @@ static int h1_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 07ccf85485e0c04c0138b9e376986bf2fc143f1d..fdbf9613b5dfa29a9fba471ce5eae37a0cbe7727 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -374,11 +374,16 @@ static struct h2_counters {
 static int h2_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                          struct field *stats, unsigned int *selected_field)
 {
-       struct h2_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < H2_STATS_COUNT; current_field++) {
                struct field metric = { 0 };
+               struct h2_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                case H2_ST_HEADERS_RCVD:
@@ -428,6 +433,7 @@ static int h2_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;

diff --git a/src/quic_stats.c b/src/quic_stats.c
index 3f2b5394165a1467d5d5014ce9fc681d7b45e6e1..0dc6b632d2ecaca03850fc3cf6c69fd02962765d 100644 (file)
--- a/src/quic_stats.c
+++ b/src/quic_stats.c
@@ -95,11 +95,16 @@ struct quic_counters quic_counters;
 static int quic_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                            struct field *stats, unsigned int *selected_field)
 {
-       struct quic_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < QUIC_STATS_COUNT; current_field++) {
                struct field metric = { 0 };
+               struct quic_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                case QUIC_ST_RXBUF_FULL:
@@ -239,6 +244,7 @@ static int quic_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;

diff --git a/src/resolvers.c b/src/resolvers.c
index d653482b7d65c3719ce4e9c78ca5fd755e15c377..6cbb4448acb1599dd21e18c2f8c85784b2af5b19 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -125,11 +125,16 @@ static struct dns_counters dns_counters;
 static int resolv_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                              struct field *stats, unsigned int *selected_field)
 {
-       struct dns_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < RSLV_STAT_END; current_field++) {
                struct field metric = { 0 };
+               struct dns_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                case RSLV_STAT_ID:
@@ -191,6 +196,7 @@ static int resolv_fill_stats(struct stats_module *mod, struct extra_counters *ct
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;

diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index f841bb11c54c18fa014a064aaa32ec040a098a94..510798e5509662e426a235b6da8870c4af0d38c0 100644 (file)
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -197,11 +197,16 @@ static struct ssl_counters ssl_counters;
 static int ssl_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                           struct field *stats, unsigned int *selected_field)
 {
-       struct ssl_counters *counters = EXTRA_COUNTERS_GET(ctr, mod);
        unsigned int current_field = (selected_field != NULL ? *selected_field : 0);
 
        for (; current_field < SSL_ST_STATS_COUNT; current_field++) {
                struct field metric = { 0 };
+               struct ssl_counters *counters;
+
+               if (!ctr)
+                       goto store_metric;
+
+               counters = EXTRA_COUNTERS_GET(ctr, mod);
 
                switch (current_field) {
                case SSL_ST_SESS:
@@ -228,6 +233,7 @@ static int ssl_fill_stats(struct stats_module *mod, struct extra_counters *ctr,
                                return 0;
                        continue;
                }
+       store_metric:
                stats[current_field] = metric;
                if (selected_field != NULL)
                        break;