[Asterisk-code-review] res_prometheus: Clone containers before iterating (asterisk[18])

[Friendly Automation]

Friendly Automation has submitted this change. ( https://gerrit.asterisk.org/c/asterisk/+/15724 )

Change subject: res_prometheus: Clone containers before iterating
......................................................................

res_prometheus: Clone containers before iterating

The channels, bridges and endpoints scrape functions were
grabbing their respective global containers, getting the
count of entries, allocating metric arrays based on
that count, then iterating over the container.  If the
global container had new objects added after the count
was taken and the metric arrays were allocated, we'd run
out of metric entries and attempt to write past the end
of the arrays.

Now each of the scape functions clone their respective
global containers and all operations are done on the
clone.  Since the clone is stable between getting the
count and iterating over it, we can't run past the end
of the metrics array.

ASTERISK-29130
Reported-By: Francisco Correia
Reported-By: BJ Weschke
Reported-By: Sébastien Duthil

Change-Id: If0c8e40853bc0e9429f2ba9c7f5f358d90c311af
---
M res/prometheus/bridges.c
M res/prometheus/channels.c
M res/prometheus/endpoints.c
3 files changed, 31 insertions(+), 5 deletions(-)

Approvals:
  Joshua Colp: Looks good to me, but someone else must approve
  Kevin Harwell: Looks good to me, but someone else must approve
  Sean Bright: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/res/prometheus/bridges.c b/res/prometheus/bridges.c
index 47dee9a..0f09603 100644
--- a/res/prometheus/bridges.c
+++ b/res/prometheus/bridges.c
@@ -77,6 +77,7 @@
  */
 static void bridges_scrape_cb(struct ast_str **response)
 {
+	struct ao2_container *bridge_cache;
 	struct ao2_container *bridges;
 	struct ao2_iterator it_bridges;
 	struct ast_bridge *bridge;
@@ -92,10 +93,17 @@
 
 	ast_eid_to_str(eid_str, sizeof(eid_str), &ast_eid_default);
 
-	bridges = ast_bridges();
+	bridge_cache = ast_bridges();
+	if (!bridge_cache) {
+		return;
+	}
+
+	bridges = ao2_container_clone(bridge_cache, 0);
+	ao2_ref(bridge_cache, -1);
 	if (!bridges) {
 		return;
 	}
+
 	num_bridges = ao2_container_count(bridges);
 
 	/* Current endpoint count */
@@ -178,4 +186,4 @@
 	prometheus_callback_register(&bridges_callback);
 
 	return 0;
-}
\ No newline at end of file
+}
diff --git a/res/prometheus/channels.c b/res/prometheus/channels.c
index 97b7519..930ae54 100644
--- a/res/prometheus/channels.c
+++ b/res/prometheus/channels.c
@@ -129,6 +129,7 @@
  */
 static void channels_scrape_cb(struct ast_str **response)
 {
+	struct ao2_container *channel_cache;
 	struct ao2_container *channels;
 	struct ao2_iterator it_chans;
 	struct ast_channel_snapshot *snapshot;
@@ -145,7 +146,17 @@
 
 	ast_eid_to_str(eid_str, sizeof(eid_str), &ast_eid_default);
 
-	channels = ast_channel_cache_all();
+	channel_cache = ast_channel_cache_all();
+	if (!channel_cache) {
+		return;
+	}
+
+	channels = ao2_container_clone(channel_cache, 0);
+	ao2_ref(channel_cache, -1);
+	if (!channels) {
+		return;
+	}
+
 	num_channels = ao2_container_count(channels);
 
 	/* Channel count */
@@ -233,4 +244,4 @@
 	prometheus_callback_register(&channels_callback);
 
 	return 0;
-}
\ No newline at end of file
+}
diff --git a/res/prometheus/endpoints.c b/res/prometheus/endpoints.c
index 5f758c5..a575198 100644
--- a/res/prometheus/endpoints.c
+++ b/res/prometheus/endpoints.c
@@ -96,6 +96,7 @@
  */
 static void endpoints_scrape_cb(struct ast_str **response)
 {
+	struct ao2_container *endpoint_cache;
 	struct ao2_container *endpoints;
 	struct ao2_iterator it_endpoints;
 	struct stasis_message *message;
@@ -111,10 +112,16 @@
 
 	ast_eid_to_str(eid_str, sizeof(eid_str), &ast_eid_default);
 
-	endpoints = stasis_cache_dump(ast_endpoint_cache(), ast_endpoint_snapshot_type());
+	endpoint_cache = stasis_cache_dump(ast_endpoint_cache(), ast_endpoint_snapshot_type());
+	if (!endpoint_cache) {
+		return;
+	}
+	endpoints = ao2_container_clone(endpoint_cache, 0);
+	ao2_ref(endpoint_cache, -1);
 	if (!endpoints) {
 		return;
 	}
+
 	num_endpoints = ao2_container_count(endpoints);
 
 	/* Current endpoint count */

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/15724
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 18
Gerrit-Change-Id: If0c8e40853bc0e9429f2ba9c7f5f358d90c311af
Gerrit-Change-Number: 15724
Gerrit-PatchSet: 3
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Sean Bright <sean.bright at gmail.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20210402/5b3c98d2/attachment-0001.html>