diff --git a/backport-coroparse-enhancements.patch b/backport-coroparse-enhancements.patch new file mode 100644 index 0000000000000000000000000000000000000000..eb2b68a531beed592ead102f5cb8166ee5bcc1e7 --- /dev/null +++ b/backport-coroparse-enhancements.patch @@ -0,0 +1,1029 @@ +From ff1039960a7dae2985b4ba682c5b62440ed88dec Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 17:01:06 +0200 +Subject: [PATCH 01/13] coroparse: Check emptiness of key name + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 6f4adf87..d4738b73 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -411,6 +411,11 @@ static int parse_section(FILE *fp, + key = remove_whitespace(line, 1); + value = remove_whitespace(loc, 0); + ++ if (strlen(key) == 0) { ++ tmp_error_string = "Key name can't be empty"; ++ goto parse_error; ++ } ++ + if (strlen(path) + strlen(key) + 1 >= ICMAP_KEYNAME_MAXLEN) { + tmp_error_string = "New key makes total cmap path too long"; + goto parse_error; +-- +2.25.1 + +From d2983df7ec2c27a70cb070507a23303f67a60504 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 16:32:43 +0200 +Subject: [PATCH 02/13] coroparse: Mark path in parse_section as const + +It's expected parse_secion doesn't change path. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index d4738b73..feb5bfce 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -291,7 +291,7 @@ static char *remove_whitespace(char *string, int remove_colon_and_brace) + static int parse_section(FILE *fp, + const char *fname, + int *line_no, +- char *path, ++ const char *path, + const char **error_string, + int depth, + enum main_cp_cb_data_state state, +-- +2.25.1 + +From 68c8086ada2a05b7961b1dc82621410b3b941c6a Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 16:54:00 +0200 +Subject: [PATCH 03/13] coroparse: Remove unused code + +Setting state for RESOURCES end of section is not needed because +of aab55a004bb12ebe78db341dc56759dfe710c1b2. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 10 ---------- + 1 file changed, 10 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index feb5bfce..966f43c7 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -1460,21 +1460,11 @@ static int main_config_parser_cb(const char *path, + case MAIN_CP_CB_DATA_STATE_NODELIST: + case MAIN_CP_CB_DATA_STATE_TOTEM: + case MAIN_CP_CB_DATA_STATE_SYSTEM: +- break; + case MAIN_CP_CB_DATA_STATE_RESOURCES: +- *state = MAIN_CP_CB_DATA_STATE_NORMAL; +- break; + case MAIN_CP_CB_DATA_STATE_RESOURCES_SYSTEM: +- *state = MAIN_CP_CB_DATA_STATE_RESOURCES; +- break; + case MAIN_CP_CB_DATA_STATE_RESOURCES_SYSTEM_MEMUSED: +- *state = MAIN_CP_CB_DATA_STATE_RESOURCES_SYSTEM; +- break; + case MAIN_CP_CB_DATA_STATE_RESOURCES_PROCESS: +- *state = MAIN_CP_CB_DATA_STATE_RESOURCES; +- break; + case MAIN_CP_CB_DATA_STATE_RESOURCES_PROCESS_MEMUSED: +- *state = MAIN_CP_CB_DATA_STATE_RESOURCES_PROCESS; + break; + } + break; +-- +2.25.1 + +From da5833e32eea51a0be6fff31e4238c6ad6378f2a Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 16:33:40 +0200 +Subject: [PATCH 04/13] coroparse: Handle end of special sections + +Special sections totem.interface, logging.logger_subsys, +logging.logging_daemon and nodelist.node executes extra code on end of +section. Previously, any end of section triggered handling, including +subsection one. + +So for example config like: +``` +nodelist { + node { + name: node1 + subs { + key: val + } + } + + node { + name: node2 + ... +``` + +result in node_number increased twice (once for end of "subs" section +and second time for end of "node" section) so node2 got number 2 instead +of 1. + +Same was happening for all other special sections and may result in +crash as reported by vikk777 in +issues #783 and #784. + +Solution is to execute extra code only for real end of section and not +for subsection. + +This patch fixes only main problem but it creates keys without +subsection name what is not optimal. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 28 ++++++++++++++++++++++++++++ + 1 file changed, 28 insertions(+) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 966f43c7..a3286b81 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -1206,6 +1206,13 @@ static int main_config_parser_cb(const char *path, + case PARSER_CB_SECTION_END: + switch (*state) { + case MAIN_CP_CB_DATA_STATE_INTERFACE: ++ if (strcmp(path, "totem.interface") != 0) { ++ /* ++ * Process only end of totem.interface section, not subsections ++ */ ++ break; ++ } ++ + /* + * Create new interface section + */ +@@ -1336,6 +1343,13 @@ static int main_config_parser_cb(const char *path, + + break; + case MAIN_CP_CB_DATA_STATE_LOGGER_SUBSYS: ++ if (strcmp(path, "logging.logger_subsys") != 0) { ++ /* ++ * Process only end of logging.logger_subsys section, not subsections ++ */ ++ break; ++ } ++ + if (data->subsys == NULL) { + *error_string = "No subsys key in logger_subsys directive"; + +@@ -1369,6 +1383,13 @@ static int main_config_parser_cb(const char *path, + } + break; + case MAIN_CP_CB_DATA_STATE_LOGGING_DAEMON: ++ if (strcmp(path, "logging.logging_daemon") != 0) { ++ /* ++ * Process only end of logging.logging_daemon section, not subsections ++ */ ++ break; ++ } ++ + if (data->logging_daemon_name == NULL) { + *error_string = "No name key in logging_daemon directive"; + +@@ -1449,6 +1470,13 @@ static int main_config_parser_cb(const char *path, + } + break; + case MAIN_CP_CB_DATA_STATE_NODELIST_NODE: ++ if (strcmp(path, "nodelist.node") != 0) { ++ /* ++ * Process only end of nodelist.node section, not subsections ++ */ ++ break; ++ } ++ + data->node_number++; + break; + case MAIN_CP_CB_DATA_STATE_NORMAL: +-- +2.25.1 + +From ee8f6e02a14b6e23718e98820e0f9de77f4370d2 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 18:29:46 +0200 +Subject: [PATCH 05/13] coroparse: Store subsections of logger_subsys + +Store full path instead of just key name. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 13 +++++++++++-- + 1 file changed, 11 insertions(+), 2 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index a3286b81..d5f58435 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -608,6 +608,7 @@ static int main_config_parser_cb(const char *path, + struct qb_list_head *iter, *tmp_iter; + int uid, gid; + cs_error_t cs_err; ++ const char *path_prefix; + + cs_err = CS_OK; + +@@ -940,7 +941,7 @@ static int main_config_parser_cb(const char *path, + } + break; + case MAIN_CP_CB_DATA_STATE_LOGGER_SUBSYS: +- if (strcmp(key, "subsys") == 0) { ++ if (strcmp(path, "logging.logger_subsys.subsys") == 0) { + data->subsys = strdup(value); + if (data->subsys == NULL) { + *error_string = "Can't alloc memory"; +@@ -948,6 +949,14 @@ static int main_config_parser_cb(const char *path, + return (0); + } + } else { ++ path_prefix = "logging.logger_subsys."; ++ if (strlen(path) < strlen(path_prefix) || ++ strncmp(path, path_prefix, strlen(path_prefix)) != 0) { ++ *error_string = "Internal error - incorrect path prefix for logger subsys state"; ++ ++ return (0); ++ } ++ + kv_item = malloc(sizeof(*kv_item)); + if (kv_item == NULL) { + *error_string = "Can't alloc memory"; +@@ -956,7 +965,7 @@ static int main_config_parser_cb(const char *path, + } + memset(kv_item, 0, sizeof(*kv_item)); + +- kv_item->key = strdup(key); ++ kv_item->key = strdup(path + strlen(path_prefix)); + kv_item->value = strdup(value); + if (kv_item->key == NULL || kv_item->value == NULL) { + free(kv_item->key); +-- +2.25.1 + +From e40a570f52482297ff4f4e4187b6144f19a146aa Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Thu, 24 Apr 2025 18:30:37 +0200 +Subject: [PATCH 06/13] coroparse: Store subsections of logging_daemon + +Store full path instead of just key name. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index d5f58435..934660de 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -981,14 +981,14 @@ static int main_config_parser_cb(const char *path, + add_as_string = 0; + break; + case MAIN_CP_CB_DATA_STATE_LOGGING_DAEMON: +- if (strcmp(key, "subsys") == 0) { ++ if (strcmp(path, "logging.logging_daemon.subsys") == 0) { + data->subsys = strdup(value); + if (data->subsys == NULL) { + *error_string = "Can't alloc memory"; + + return (0); + } +- } else if (strcmp(key, "name") == 0) { ++ } else if (strcmp(path, "logging.logging_daemon.name") == 0) { + data->logging_daemon_name = strdup(value); + if (data->logging_daemon_name == NULL) { + *error_string = "Can't alloc memory"; +@@ -996,6 +996,14 @@ static int main_config_parser_cb(const char *path, + return (0); + } + } else { ++ path_prefix = "logging.logging_daemon."; ++ if (strlen(path) < strlen(path_prefix) || ++ strncmp(path, path_prefix, strlen(path_prefix)) != 0) { ++ *error_string = "Internal error - incorrect path prefix for logging daemon state"; ++ ++ return (0); ++ } ++ + kv_item = malloc(sizeof(*kv_item)); + if (kv_item == NULL) { + *error_string = "Can't alloc memory"; +@@ -1004,7 +1012,7 @@ static int main_config_parser_cb(const char *path, + } + memset(kv_item, 0, sizeof(*kv_item)); + +- kv_item->key = strdup(key); ++ kv_item->key = strdup(path + strlen(path_prefix)); + kv_item->value = strdup(value); + if (kv_item->key == NULL || kv_item->value == NULL) { + free(kv_item->key); +-- +2.25.1 + +From a14311a5a4ce7249351fcc042b7435ee1b69caa4 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 08:55:24 +0200 +Subject: [PATCH 07/13] coroparse: Don't allow sections within uidgid + +Unify behavior with uidgid files and solve problem with incorrect prefix +if subsections were used. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 934660de..3ee5266c 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -1028,7 +1028,7 @@ static int main_config_parser_cb(const char *path, + add_as_string = 0; + break; + case MAIN_CP_CB_DATA_STATE_UIDGID: +- if (strcmp(key, "uid") == 0) { ++ if (strcmp(path, "uidgid.uid") == 0) { + uid = uid_determine(value); + if (uid == -1) { + *error_string = error_string_response; +@@ -1040,7 +1040,7 @@ static int main_config_parser_cb(const char *path, + goto icmap_set_error; + } + add_as_string = 0; +- } else if (strcmp(key, "gid") == 0) { ++ } else if (strcmp(path, "uidgid.gid") == 0) { + gid = gid_determine(value); + if (gid == -1) { + *error_string = error_string_response; +@@ -1219,6 +1219,12 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "resources.process.memory_used") == 0) { + *state = MAIN_CP_CB_DATA_STATE_RESOURCES_PROCESS_MEMUSED; + } ++ ++ if (*state == MAIN_CP_CB_DATA_STATE_UIDGID && strcmp(path, "uidgid") != 0) { ++ *error_string = "Subsections are not allowed within uidgid section"; ++ ++ return (0); ++ }; + break; + case PARSER_CB_SECTION_END: + switch (*state) { +-- +2.25.1 + +From 8100d953e05a52b7d98ff6ad99a5b02d0ae6fcb9 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 09:01:31 +0200 +Subject: [PATCH 08/13] coroparse: Don't allow sections within member + +Solve problem with incorrect prefix if subsections were used. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 8 +++++++- + 1 file changed, 7 insertions(+), 1 deletion(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 3ee5266c..a385ce0a 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -1058,7 +1058,7 @@ static int main_config_parser_cb(const char *path, + } + break; + case MAIN_CP_CB_DATA_STATE_MEMBER: +- if (strcmp(key, "memberaddr") != 0) { ++ if (strcmp(path, "totem.interface.member.memberaddr") != 0) { + *error_string = "Only memberaddr is allowed in member section"; + + return (0); +@@ -1225,6 +1225,12 @@ static int main_config_parser_cb(const char *path, + + return (0); + }; ++ ++ if (*state == MAIN_CP_CB_DATA_STATE_MEMBER && strcmp(path, "totem.interface.member") != 0) { ++ *error_string = "Subsections are not allowed within totem.interface.member section"; ++ ++ return (0); ++ }; + break; + case PARSER_CB_SECTION_END: + switch (*state) { +-- +2.25.1 + +From 62cbfb3d6d813804dee35c4afc5dbf693ef3fdf9 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 09:25:31 +0200 +Subject: [PATCH 09/13] coroparse: Store key with prefix for nodelist.node + +Config file like: +``` +nodelist { + node { + nodeid: 1 + subsection { + nodeid: 2 + } + } +... +``` +was parsed incorrectly and subsection nodeid (2) was used instead of +node section nodeid (1). + +Solution is to properly check key path instead of just key name and +use key path - "nodelist.node." prefix as new key name. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 15 ++++++++++++--- + 1 file changed, 12 insertions(+), 3 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index a385ce0a..96b4f9bd 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -1089,9 +1089,18 @@ static int main_config_parser_cb(const char *path, + case MAIN_CP_CB_DATA_STATE_NODELIST: + break; + case MAIN_CP_CB_DATA_STATE_NODELIST_NODE: +- snprintf(key_name, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.%s", data->node_number, key); +- if ((strcmp(key, "nodeid") == 0) || +- (strcmp(key, "quorum_votes") == 0)) { ++ path_prefix = "nodelist.node."; ++ if (strlen(path) < strlen(path_prefix) || ++ strncmp(path, path_prefix, strlen(path_prefix)) != 0) { ++ *error_string = "Internal error - incorrect path prefix for nodelist node state"; ++ ++ return (0); ++ } ++ ++ snprintf(key_name, ICMAP_KEYNAME_MAXLEN, "nodelist.node.%u.%s", data->node_number, ++ path + strlen(path_prefix)); ++ if ((strcmp(path, "nodelist.node.nodeid") == 0) || ++ (strcmp(path, "nodelist.node.quorum_votes") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { + goto atoi_error; +-- +2.25.1 + +From efb3bc2b06a180f22650bc30515670b6ec1a7d08 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 10:24:41 +0200 +Subject: [PATCH 10/13] coroparse: Fix memory leaks + +Previously, when parsing error happened, parser callback data were not +freed. This was not a big problem when reload was not implemented, but +it might be problem with reload. + +Solution is to add new callback type PARSER_CB_CLEANUP which is called +either on error or end of parsing if there is no error. Callback is +responsible for cleaning all allocated memory. + +To make such callback work reliably, all variables must be set to NULL +on cleanup (example is data->subsys) and linked list must be +reinitialized. + +Another source of possible leak is strdup of some keys in +(like totem.interface.bindnetaddr, but there is +more similar examples) without previously freeing it. +This is problem if bindnetaddr is defined multiple times. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 50 ++++++++++++++++++++++++++++++++++++++++++++---- + 1 file changed, 46 insertions(+), 4 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 96b4f9bd..fafec606 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -70,6 +70,7 @@ enum parser_cb_type { + PARSER_CB_SECTION_START, + PARSER_CB_SECTION_END, + PARSER_CB_ITEM, ++ PARSER_CB_CLEANUP, + }; + + enum main_cp_cb_data_state { +@@ -471,6 +472,7 @@ static int parse_section(FILE *fp, + + if (strcmp(path, "") == 0) { + parser_cb("", NULL, NULL, &state, PARSER_CB_END, error_string, config_map, user_data); ++ parser_cb("", NULL, NULL, &state, PARSER_CB_CLEANUP, error_string, config_map, user_data); + } + + return 0; +@@ -483,6 +485,8 @@ parse_error: + *error_string = formated_err; + } + ++ parser_cb("", NULL, NULL, &state, PARSER_CB_CLEANUP, error_string, config_map, user_data); ++ + return -1; + } + +@@ -637,6 +641,31 @@ static int main_config_parser_cb(const char *path, + break; + case PARSER_CB_END: + break; ++ case PARSER_CB_CLEANUP: ++ free(data->bindnetaddr); ++ free(data->mcastaddr); ++ free(data->broadcast); ++ free(data->knet_transport); ++ ++ qb_list_for_each_safe(iter, tmp_iter, &(data->logger_subsys_items_head)) { ++ kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ ++ free(kv_item->value); ++ free(kv_item->key); ++ free(kv_item); ++ } ++ ++ free(data->subsys); ++ free(data->logging_daemon_name); ++ ++ qb_list_for_each_safe(iter, tmp_iter, &(data->member_items_head)) { ++ kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ ++ free(kv_item->value); ++ free(kv_item->key); ++ free(kv_item); ++ } ++ break; + case PARSER_CB_ITEM: + add_as_string = 1; + +@@ -867,14 +896,17 @@ static int main_config_parser_cb(const char *path, + add_as_string = 0; + } + if (strcmp(path, "totem.interface.bindnetaddr") == 0) { ++ free(data->bindnetaddr); + data->bindnetaddr = strdup(value); + add_as_string = 0; + } + if (strcmp(path, "totem.interface.mcastaddr") == 0) { ++ free(data->mcastaddr); + data->mcastaddr = strdup(value); + add_as_string = 0; + } + if (strcmp(path, "totem.interface.broadcast") == 0) { ++ free(data->broadcast); + data->broadcast = strdup(value); + add_as_string = 0; + } +@@ -935,13 +967,14 @@ static int main_config_parser_cb(const char *path, + add_as_string = 0; + } + if (strcmp(path, "totem.interface.knet_transport") == 0) { +- val_type = ICMAP_VALUETYPE_STRING; ++ free(data->knet_transport); + data->knet_transport = strdup(value); + add_as_string = 0; + } + break; + case MAIN_CP_CB_DATA_STATE_LOGGER_SUBSYS: + if (strcmp(path, "logging.logger_subsys.subsys") == 0) { ++ free(data->subsys); + data->subsys = strdup(value); + if (data->subsys == NULL) { + *error_string = "Can't alloc memory"; +@@ -982,6 +1015,7 @@ static int main_config_parser_cb(const char *path, + break; + case MAIN_CP_CB_DATA_STATE_LOGGING_DAEMON: + if (strcmp(path, "logging.logging_daemon.subsys") == 0) { ++ free(data->subsys); + data->subsys = strdup(value); + if (data->subsys == NULL) { + *error_string = "Can't alloc memory"; +@@ -989,6 +1023,7 @@ static int main_config_parser_cb(const char *path, + return (0); + } + } else if (strcmp(path, "logging.logging_daemon.name") == 0) { ++ free(data->logging_daemon_name); + data->logging_daemon_name = strdup(value); + if (data->logging_daemon_name == NULL) { + *error_string = "Can't alloc memory"; +@@ -1354,6 +1389,7 @@ static int main_config_parser_cb(const char *path, + data->linknumber); + cs_err = icmap_set_string_r(config_map, key_name, data->knet_transport); + free(data->knet_transport); ++ data->knet_transport = NULL; + + if (cs_err != CS_OK) { + goto icmap_set_error; +@@ -1379,6 +1415,8 @@ static int main_config_parser_cb(const char *path, + } + } + ++ qb_list_init(&data->member_items_head); ++ + break; + case MAIN_CP_CB_DATA_STATE_LOGGER_SUBSYS: + if (strcmp(path, "logging.logger_subsys") != 0) { +@@ -1414,7 +1452,9 @@ static int main_config_parser_cb(const char *path, + data->subsys); + cs_err = icmap_set_string_r(config_map, key_name, data->subsys); + ++ qb_list_init(&data->logger_subsys_items_head); + free(data->subsys); ++ data->subsys = NULL; + + if (cs_err != CS_OK) { + goto icmap_set_error; +@@ -1489,9 +1529,6 @@ static int main_config_parser_cb(const char *path, + cs_err = icmap_set_string_r(config_map, key_name, data->subsys); + + if (cs_err != CS_OK) { +- free(data->subsys); +- free(data->logging_daemon_name); +- + goto icmap_set_error; + } + snprintf(key_name, ICMAP_KEYNAME_MAXLEN, "logging.logging_daemon.%s.%s.name", +@@ -1500,8 +1537,11 @@ static int main_config_parser_cb(const char *path, + } + } + ++ qb_list_init(&data->logger_subsys_items_head); + free(data->subsys); ++ data->subsys = NULL; + free(data->logging_daemon_name); ++ data->logging_daemon_name = NULL; + + if (cs_err != CS_OK) { + goto icmap_set_error; +@@ -1587,6 +1627,8 @@ static int uidgid_config_parser_cb(const char *path, + break; + case PARSER_CB_END: + break; ++ case PARSER_CB_CLEANUP: ++ break; + case PARSER_CB_ITEM: + if (strcmp(path, "uidgid.uid") == 0) { + uid = uid_determine(value); +-- +2.25.1 + +From 10cf367e9fd6152f98a7471f0882871d2041efab Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 13:17:15 +0200 +Subject: [PATCH 11/13] coroparse: Initialize logger_subsys_items_head + +Using memset is not a valid initialization method for list so use +qb_list_init. + +Big thanks for vikk777 for test and +reporting the problem. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 2 ++ + 1 file changed, 2 insertions(+) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index fafec606..76f86c55 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -637,6 +637,8 @@ static int main_config_parser_cb(const char *path, + switch (type) { + case PARSER_CB_START: + memset(data, 0, sizeof(struct main_cp_cb_data)); ++ qb_list_init(&data->logger_subsys_items_head); ++ qb_list_init(&data->member_items_head); + *state = MAIN_CP_CB_DATA_STATE_NORMAL; + break; + case PARSER_CB_END: +-- +2.25.1 + +From e321d1b233ce1a57063da7e3dad94a0b2de1842e Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 15:48:54 +0200 +Subject: [PATCH 12/13] coroparse: Remove kv_items from list + +qb_list_init is called only on successful list traversal and without +removing successfully processed items cleanup access them again +resulting in use-after-free error. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 76f86c55..0a1eaa2f 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -651,6 +651,7 @@ static int main_config_parser_cb(const char *path, + + qb_list_for_each_safe(iter, tmp_iter, &(data->logger_subsys_items_head)) { + kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ qb_list_del(&kv_item->list); + + free(kv_item->value); + free(kv_item->key); +@@ -662,6 +663,7 @@ static int main_config_parser_cb(const char *path, + + qb_list_for_each_safe(iter, tmp_iter, &(data->member_items_head)) { + kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ qb_list_del(&kv_item->list); + + free(kv_item->value); + free(kv_item->key); +@@ -1402,6 +1404,7 @@ static int main_config_parser_cb(const char *path, + + qb_list_for_each_safe(iter, tmp_iter, &(data->member_items_head)) { + kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ qb_list_del(&kv_item->list); + + snprintf(key_name, ICMAP_KEYNAME_MAXLEN, "totem.interface.%u.member.%u", + data->linknumber, ii); +@@ -1436,6 +1439,7 @@ static int main_config_parser_cb(const char *path, + + qb_list_for_each_safe(iter, tmp_iter, &(data->logger_subsys_items_head)) { + kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ qb_list_del(&kv_item->list); + + snprintf(key_name, ICMAP_KEYNAME_MAXLEN, "logging.logger_subsys.%s.%s", + data->subsys, kv_item->key); +@@ -1478,6 +1482,7 @@ static int main_config_parser_cb(const char *path, + + qb_list_for_each_safe(iter, tmp_iter, &(data->logger_subsys_items_head)) { + kv_item = qb_list_entry(iter, struct key_value_list_item, list); ++ qb_list_del(&kv_item->list); + + if (data->subsys == NULL) { + if (strcmp(data->logging_daemon_name, "corosync") == 0) { +-- +2.25.1 + +From 7e643af101ca881447a9ea5e4b5eb2d6787c5128 Mon Sep 17 00:00:00 2001 +From: Jan Friesse +Date: Fri, 25 Apr 2025 16:13:24 +0200 +Subject: [PATCH 13/13] coroparse: Implement handler for str_to_ull error + +Previously atoi_error handler was used for both safe_atoq and str_to_ull +errors. This is wrong, because str_to_ull doesn't define val_type so +safe_atoq_range assert either failed or error message contained invalid +integer ranges. + +Example of such file is +``` +totem { + version: 2 + config_version: c42 +... +``` + +which results in abort. + +Solution is to split safe_atoq and str_to_ull error handling, first one +displaying range and second one doesn't. + +Signed-off-by: Jan Friesse +Reviewed-by: Christine Caulfield +--- + exec/coroparse.c | 59 +++++++++++++++++++++++++++++++----------------- + 1 file changed, 38 insertions(+), 21 deletions(-) + +diff --git a/exec/coroparse.c b/exec/coroparse.c +index 0a1eaa2f..e90e9b80 100644 +--- a/exec/coroparse.c ++++ b/exec/coroparse.c +@@ -681,7 +681,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "pload.size") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint32_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -696,7 +696,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "quorum.leaving_timeout") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint32_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -712,7 +712,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "quorum.last_man_standing") == 0)) { + val_type = ICMAP_VALUETYPE_UINT8; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint8_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -726,7 +726,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "quorum.device.votes") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint32_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -736,7 +736,7 @@ static int main_config_parser_cb(const char *path, + if ((strcmp(path, "quorum.device.master_wins") == 0)) { + val_type = ICMAP_VALUETYPE_UINT8; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint8_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -777,7 +777,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "totem.netmtu") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint32_r(config_map,path, val)) != CS_OK) { + goto icmap_set_error; +@@ -787,7 +787,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.knet_compression_level") == 0) { + val_type = ICMAP_VALUETYPE_INT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_int32_r(config_map, path, val)) != CS_OK) { + goto icmap_set_error; +@@ -796,7 +796,7 @@ static int main_config_parser_cb(const char *path, + } + if (strcmp(path, "totem.config_version") == 0) { + if (str_to_ull(value, &ull) != 0) { +- goto atoi_error; ++ goto str_to_ull_error; + } + if ((cs_err = icmap_set_uint64_r(config_map, path, ull)) != CS_OK) { + goto icmap_set_error; +@@ -893,7 +893,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.linknumber") == 0) { + val_type = ICMAP_VALUETYPE_UINT8; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + + data->linknumber = val; +@@ -917,7 +917,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.mcastport") == 0) { + val_type = ICMAP_VALUETYPE_UINT16; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->mcastport = val; + add_as_string = 0; +@@ -925,7 +925,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.ttl") == 0) { + val_type = ICMAP_VALUETYPE_UINT8; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->ttl = val; + add_as_string = 0; +@@ -933,7 +933,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.knet_link_priority") == 0) { + val_type = ICMAP_VALUETYPE_UINT8; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->knet_link_priority = val; + add_as_string = 0; +@@ -941,7 +941,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.knet_ping_interval") == 0) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->knet_ping_interval = val; + add_as_string = 0; +@@ -949,7 +949,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.knet_ping_timeout") == 0) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->knet_ping_timeout = val; + add_as_string = 0; +@@ -957,7 +957,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.knet_ping_precision") == 0) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->knet_ping_precision = val; + add_as_string = 0; +@@ -965,7 +965,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(path, "totem.interface.knet_pong_count") == 0) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + data->knet_pong_count = val; + add_as_string = 0; +@@ -1142,7 +1142,7 @@ static int main_config_parser_cb(const char *path, + (strcmp(path, "nodelist.node.quorum_votes") == 0)) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + + if ((cs_err = icmap_set_uint32_r(config_map, key_name, val)) != CS_OK) { +@@ -1162,7 +1162,7 @@ static int main_config_parser_cb(const char *path, + if (strcmp(key, "watchdog_timeout") == 0) { + val_type = ICMAP_VALUETYPE_UINT32; + if (safe_atoq(value, &val, val_type) != 0) { +- goto atoi_error; ++ goto safe_atoq_error; + } + if ((cs_err = icmap_set_uint32_r(config_map,path, val)) != CS_OK) { + goto icmap_set_error; +@@ -1174,7 +1174,7 @@ static int main_config_parser_cb(const char *path, + case MAIN_CP_CB_DATA_STATE_RESOURCES_SYSTEM_MEMUSED: + if (strcmp(key, "poll_period") == 0) { + if (str_to_ull(value, &ull) != 0) { +- goto atoi_error; ++ goto str_to_ull_error; + } + if ((cs_err = icmap_set_uint64_r(config_map,path, ull)) != CS_OK) { + goto icmap_set_error; +@@ -1186,7 +1186,7 @@ static int main_config_parser_cb(const char *path, + case MAIN_CP_CB_DATA_STATE_RESOURCES_PROCESS_MEMUSED: + if (strcmp(key, "poll_period") == 0) { + if (str_to_ull(value, &ull) != 0) { +- goto atoi_error; ++ goto str_to_ull_error; + } + if ((cs_err = icmap_set_uint64_r(config_map,path, ull)) != CS_OK) { + goto icmap_set_error; +@@ -1585,7 +1585,10 @@ static int main_config_parser_cb(const char *path, + + return (1); + +-atoi_error: ++safe_atoq_error: ++ /* ++ * For integers supported by safe_atoq display range ++ */ + min_val = max_val = 0; + /* + * This is really assert, because developer ether doesn't set val_type correctly or +@@ -1603,6 +1606,20 @@ atoi_error: + + return (0); + ++str_to_ull_error: ++ /* ++ * For integers not supported by safe_atoq (64-bit int) ++ */ ++ if (snprintf(formated_err, sizeof(formated_err), ++ "Value of key \"%s\" is expected to be unsigned integer, but \"%s\" was given", ++ key_name, value) >= sizeof(formated_err)) { ++ *error_string = "Can't format parser error message"; ++ } else { ++ *error_string = formated_err; ++ } ++ ++ return (0); ++ + icmap_set_error: + if (snprintf(formated_err, sizeof(formated_err), + "Can't store key \"%s\" into icmap, returned error is %s", +-- +2.25.1 + + + diff --git a/corosync.spec b/corosync.spec index 62c6c40530c2ffc64e90b9be9477e702fdc185be..0ef4a1470a4f8c0665435ca119a7a891bef785a2 100644 --- a/corosync.spec +++ b/corosync.spec @@ -18,12 +18,13 @@ Name: corosync Summary: The Corosync Cluster Engine and Application Programming Interfaces Version: 3.1.9 -Release: 2 +Release: 3 License: BSD-3-Clause URL: http://corosync.github.io/corosync/ Source0: http://build.clusterlabs.org/corosync/releases/%{name}-%{version}%{?gittarver}.tar.gz Patch0: backport-CVE-2025-30472.patch Patch1: backport-Fix-stack-buffer-overflow-in-remove_whitespace.patch +Patch2: backport-coroparse-enhancements.patch # Runtime bits # The automatic dependency overridden in favor of explicit version lock Requires: corosynclib = %{version}-%{release} @@ -289,6 +290,11 @@ network splits) %endif %changelog +* Wed Aug 20 2025 zouzhimin - 3.1.9-3 +- Coroparse enhancements: +- fix multiple memory leaks in coroparse.c while fuzzing project +- fix heap use after free in coroparse.c + * Thu Aug 14 2025 zouzhimin - 3.1.9-2 - Fix stack buffer overflow in remove_whitespace()