From 7c96bc412f9ebb3b48bd96d75598cd0d20ab60b3 Mon Sep 17 00:00:00 2001 From: fumingwei Date: Tue, 29 Aug 2023 16:27:13 +0800 Subject: bugfix:修复构建uthash metric key过程中存在的内存越界问题 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/fieldstat_dynamic.cpp | 54 +++++++- src/fieldstat_internal.h | 4 +- test/src/gtest_dynamic_fieldstat_output.cpp | 185 ++++++++++++++++++++++++++++ 3 files changed, 240 insertions(+), 3 deletions(-) diff --git a/src/fieldstat_dynamic.cpp b/src/fieldstat_dynamic.cpp index 59faba5..04efa59 100644 --- a/src/fieldstat_dynamic.cpp +++ b/src/fieldstat_dynamic.cpp @@ -243,11 +243,21 @@ static int build_dynamic_metric_key(int table_id, const char *field_name, const /* part1: field name */ unsigned int field_name_len = strlen(field_name) + 1; + + if(field_name_len > out_key_size) + { + return 0; + } + memcpy(out_key + used_len, field_name, field_name_len); //escaping_special_chars(out_key); used_len += field_name_len; /* part2: table id */ + if(used_len + sizeof(table_id) > out_key_size) + { + return 0; + } memcpy(out_key + used_len, &table_id, sizeof(table_id)); used_len += sizeof(table_id); @@ -258,20 +268,39 @@ static int build_dynamic_metric_key(int table_id, const char *field_name, const /* tag key len */ unsigned int tag_key_len = strlen(tag->key) + 1; unsigned int tag_value_len; + + if(used_len + tag_key_len > out_key_size) + { + return 0; + } memcpy(out_key + used_len, tag->key, tag_key_len); used_len += tag_key_len; switch (tag->value_type) { case 0: + if(used_len + sizeof(tag->value_int) > out_key_size) + { + return 0; + } memcpy(out_key + used_len, &tag->value_int, sizeof(tag->value_int)); used_len += sizeof(tag->value_int); break; case 1: + if(used_len + sizeof(tag->value_double) > out_key_size) + { + return 0; + } + memcpy(out_key + used_len, &tag->value_double, sizeof(tag->value_double)); used_len += sizeof(tag->value_double); break; case 2: tag_value_len = strlen(tag->value_str) + 1; + if(used_len + tag_value_len > out_key_size) + { + return 0; + } + memcpy(out_key + used_len, tag->value_str, tag_value_len); used_len += tag_value_len; break; @@ -291,9 +320,13 @@ static struct metric * read_dynamic_metric(struct fieldstat_dynamic_instance *in struct dynamic_metric **head = &instance->n_thread_dynamic_metric[thread_id]; struct dynamic_metric *find = NULL; - char dynamic_metric_key[512]; + char dynamic_metric_key[METRIC_SIZE]; unsigned int dynamic_metric_keylen = 0; dynamic_metric_keylen = build_dynamic_metric_key(table_id, field_name, tags, n_tags, sizeof(dynamic_metric_key), dynamic_metric_key); + if(dynamic_metric_keylen == 0) + { + return NULL; + } HASH_FIND(hh, *head, dynamic_metric_key, dynamic_metric_keylen, find); if(find == NULL) @@ -332,6 +365,13 @@ static struct metric * create_dynamic_table_metric(struct fieldstat_dynamic_inst unsigned int metric_keylen; metric_keylen = build_dynamic_metric_key(table_id, row_name, tags, n_tags, sizeof(value->metric_key), value->metric_key); + if(metric_keylen == 0) + { + free(value); + value = NULL; + return NULL; + } + value->metric_keylen = metric_keylen; table = instance->table_metrics[table_id]; @@ -388,6 +428,12 @@ static struct metric * create_dynamic_metric(struct fieldstat_dynamic_instance * insert = (struct dynamic_metric *)calloc(1, sizeof(struct dynamic_metric)); insert->metric_keylen = build_dynamic_metric_key(-1, field_name, tags, n_tags, sizeof(insert->metric_key), insert->metric_key); + if(insert->metric_keylen == 0) + { + free(insert); + insert = NULL; + return NULL; + } insert->metrics = (struct metric **)calloc(1, sizeof(struct metric *)); metric = metric_new(type, field_name, tags, n_tags); @@ -598,11 +644,15 @@ static int table_row_metric_values_operate( enum field_op op) { struct metric **metrics = NULL; - char metric_key[512]; + char metric_key[METRIC_SIZE]; unsigned int metric_keylen = 0; metric_keylen = build_dynamic_metric_key(table_id, row_name, tags, n_tags, sizeof(metric_key), metric_key); + if(metric_keylen == 0) + { + return -1; + } metrics = read_dynamic_row_metrics(instance, metric_key, metric_keylen, thread_id); diff --git a/src/fieldstat_internal.h b/src/fieldstat_internal.h index 08ecd2a..5e3c33b 100644 --- a/src/fieldstat_internal.h +++ b/src/fieldstat_internal.h @@ -65,6 +65,8 @@ #define PROMETHEUS_ENDPOINT_DEFAULT_URL "/metrics" #define TABLE_LINE_SCALE_NUM 1024 +#define METRIC_SIZE 1024 + enum field_calc_algo @@ -213,7 +215,7 @@ struct prometheus_endpoint_instance struct dynamic_metric { - char metric_key[512]; + char metric_key[METRIC_SIZE]; unsigned int metric_keylen; struct metric **metrics; diff --git a/test/src/gtest_dynamic_fieldstat_output.cpp b/test/src/gtest_dynamic_fieldstat_output.cpp index fab3dc8..5e86372 100644 --- a/test/src/gtest_dynamic_fieldstat_output.cpp +++ b/test/src/gtest_dynamic_fieldstat_output.cpp @@ -117,6 +117,191 @@ TEST(FeildStatDynamicAPI, FieldStatDynamicInstanceMultiIncrby) fieldstat_dynamic_instance_free(instance); } +void _worker_thread_table_incrby(void *arg) +{ + struct thread_para *para = (struct thread_para*)arg; + int loops = para->loops; + int table_id = para->table_id; + + const char *row_name = "security_rule_hits"; + struct fieldstat_dynamic_instance *instance = para->instance; + int ret = 0; + struct fieldstat_tag tags[14]; + long long values[20]; + + + tags[0].key = "policy_id"; + tags[0].value_int = 1; + tags[0].value_type = 0; + + tags[1].key = "quanlity"; + tags[1].value_double = 0.50; + tags[1].value_type = 1; + + tags[2].key = "device_id"; + tags[2].value_str = "xxg-cabinet0"; + tags[2].value_type = 2; + + tags[3].key = "device_id333333333333333333333333333333333333333333333333333" + "333333333333333333333333333333333333333"; + tags[3].value_str = "device_id_3"; + tags[3].value_type = 2; + + tags[4].key = "quanlity_444444444444444444444444444444444444444444444444444" + "444444444444444444444444444444444444444"; + tags[4].value_str = "quanlity_4"; + tags[4].value_type = 2; + + tags[5].key = "policy_id_55555555555555555555555555555555555555555555555555" + "555555555555555555555555555555555555555"; + tags[5].value_str = "policy_id_5"; + tags[5].value_type = 2; + + tags[6].key = "vsys_id_6666666666666666666666666666666666666666666666666666" + "666666666666666666666666666666666666666"; + tags[6].value_str = "vsys_id_6"; + tags[6].value_type = 2; + + tags[7].key = "system_id_77777777777777777777777777777777777777777777777777" + "777777777777777777777777777777777777777"; + tags[7].value_int = 0; + tags[7].value_type = 0; + + + tags[8].key = "vsys_id_8666666666666666666666666666666666666666666666666666" + "666666666666666666666666666666666666666"; + tags[8].value_str = "vsys_id_6"; + tags[8].value_type = 2; + + tags[9].key = "system_id_97777777777777777777777777777777777777777777777777" + "777777777777777777777777777777777777777"; + tags[9].value_int = 0; + tags[9].value_type = 0; + + tags[10].key = "vsys_id_106666666666666666666666666666666666666666666666666" + "6666666666666666666666666666666666666666"; + tags[10].value_str = "vsys_id_6"; + tags[10].value_type = 2; + + tags[11].key = "system_id_1177777777777777777777777777777777777777777777777" + "7777777777777777777777777777777777777777"; + tags[11].value_int = 0; + tags[11].value_type = 0; + + + tags[12].key = "system_id_1277777777777777777777777777777777777777777777777" + "7777777777777777777777777777777777777777"; + tags[12].value_int = 0; + tags[12].value_type = 0; + + + tags[13].key = "system_id_1377777777777777777777777777777777777777777777777" + "7777777777777777777777777777777777777777"; + tags[13].value_int = 0; + tags[13].value_type = 0; + + + for(int i = 0; i < loops; i++) + { + for(int j = 0; j < 20; j++) + { + values[j] = 0; + values[j] = j + i; + } + tags[0].value_int = i; + + if(i % 2 == 0) + { + ret = fieldstat_dynamic_table_row_metric_values_decrby( + instance, table_id, + row_name, values, + 20, tags, 14, 0); + EXPECT_EQ(-1, ret); + } + else + { + ret = fieldstat_dynamic_table_row_metric_values_decrby( + instance, table_id, + row_name, values, + 20, tags, 3, 0); + EXPECT_EQ(0, ret); + } + + usleep(1000); + } + return; +} + +void * worker_thread_table_incrby(void *arg) +{ + _worker_thread_table_incrby(arg); + usleep(1000 * 100); + return NULL; +} + +TEST(DynamicOutput, HashKeyOutOfSize) +{ + int ret = 0; + int n_thread = 1; + int n_loops = 1000; + unsigned int out_column_ids[20]; + int table_id = -1; + struct fieldstat_dynamic_instance *instance = NULL; + + const char *column_name[] = { + "packages01", "packages02", "packages03", "packages04", "packages05", + "packages06", "packages07", "packages08", "packages09", "packages10", + "packages11", "packages12", "packages13", "packages14", "packages15", + "packages16", "packages17", "packages18", "packages19", "packages20"}; + + enum field_type column_type[] = { + FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, + FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, + FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, + FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, + FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER, FIELD_TYPE_COUNTER}; + + + struct thread_para para[n_thread]; + pthread_t thread_ids[n_thread]; + + instance = fieldstat_dynamic_instance_new("firewall", n_thread); + ret = fieldstat_dynamic_set_line_protocol_server(instance, "127.0.0.1", 8700); + EXPECT_EQ(0, ret); + + table_id = fieldstat_register_dynamic_table(instance, "shaping", column_name, + column_type, 20, out_column_ids); + EXPECT_EQ(0, table_id); + + fieldstat_dynamic_set_output_interval(instance, 1); + system("cat /dev/null > /tmp/metrics.out"); + fieldstat_dynamic_instance_start(instance); + + for(int i = 0; i < n_thread; i++) + { + para[i].loops = n_loops; + para[i].instance = instance; + para[i].thread_id = i; + para[i].table_id = table_id; + para[i].out_column_ids = out_column_ids; + } + + for(int i = 0; i < n_thread; i++) + { + ret = pthread_create(&(thread_ids[i]), NULL, worker_thread_table_incrby, + &(para[i])); + EXPECT_EQ(0, ret); + } + + void *temp; + for(int i = 0; i < n_thread; i++) + { + pthread_join(thread_ids[i], (void**)&temp); + } + sleep(6); + fieldstat_dynamic_instance_free(instance); +} + int main(int argc, char *argv[]) { testing::InitGoogleTest(&argc, argv); -- cgit v1.2.3