Skip to content

Commit

Permalink
res_prometheus: Clone containers before iterating
Browse files Browse the repository at this point in the history
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
  • Loading branch information
gtjoseph authored and Friendly Automation committed Apr 2, 2021
1 parent 46ed6af commit 53c702e
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 5 deletions.
12 changes: 10 additions & 2 deletions res/prometheus/bridges.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ struct bridge_metric_defs {
*/
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;
Expand All @@ -92,10 +93,17 @@ static void bridges_scrape_cb(struct ast_str **response)

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 */
Expand Down Expand Up @@ -178,4 +186,4 @@ int bridge_metrics_init(void)
prometheus_callback_register(&bridges_callback);

return 0;
}
}
15 changes: 13 additions & 2 deletions res/prometheus/channels.c
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ static struct prometheus_metric global_channel_metrics[] = {
*/
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;
Expand All @@ -145,7 +146,17 @@ static void channels_scrape_cb(struct ast_str **response)

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 */
Expand Down Expand Up @@ -233,4 +244,4 @@ int channel_metrics_init(void)
prometheus_callback_register(&channels_callback);

return 0;
}
}
9 changes: 8 additions & 1 deletion res/prometheus/endpoints.c
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ struct endpoint_metric_defs {
*/
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;
Expand All @@ -111,10 +112,16 @@ static void endpoints_scrape_cb(struct ast_str **response)

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 */
Expand Down

0 comments on commit 53c702e

Please sign in to comment.