From f5d71057496b226cdbe2d5dc50c3019bf97570f5 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Tue, 27 Jan 2026 23:29:57 +0000 Subject: [PATCH 1/3] DAOS-18527 cart: Handling of string env limits - CRT_ENV_STR_MAX_SIZE introduced, set to 1024. All string envs are checked against this limit at the data_init() time. - Additional checks added for domain/interface not to exceed this limit, as those can be passed via crt_init_opt. Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 51 ++++++++++++++++++++++++++--------- src/cart/crt_internal_types.h | 50 +++++++++++++++++++++++++++++----- 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index 21f9ea08891..ace5b3ba479 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -1,7 +1,7 @@ /* * (C) Copyright 2016-2024 Intel Corporation. * (C) Copyright 2025 Google LLC - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -274,6 +274,9 @@ data_init(int server, crt_init_options_t *opt) crt_env_dump(); + if (!crt_env_list_valid()) + return -DER_INVAL; + /* Set context post init / post incr to tune number of pre-posted recvs */ crt_env_get(D_POST_INIT, &post_init); crt_gdata.cg_post_init = post_init; @@ -440,7 +443,7 @@ __split_arg(char *s_arg_to_split, const char *delim, char **first_arg, char **se return DER_SUCCESS; } - D_STRNDUP(arg_to_split, s_arg_to_split, 255); + D_STRNDUP(arg_to_split, s_arg_to_split, CRT_ENV_STR_MAX_SIZE); if (!arg_to_split) { *first_arg = NULL; *second_arg = NULL; @@ -793,25 +796,25 @@ crt_init_opt(crt_group_id_t grpid, uint32_t flags, crt_init_options_t *opt) * and processed in crt_na_config_init(). */ if (interface) { - D_STRNDUP(iface0, interface, 255); + D_STRNDUP(iface0, interface, CRT_ENV_STR_MAX_SIZE); if (!iface0) D_GOTO(unlock, rc = -DER_NOMEM); } if (domain) { - D_STRNDUP(domain0, domain, 255); + D_STRNDUP(domain0, domain, CRT_ENV_STR_MAX_SIZE); if (!domain0) D_GOTO(unlock, rc = -DER_NOMEM); } if (port) { - D_STRNDUP(port0, port, 255); + D_STRNDUP(port0, port, CRT_ENV_STR_MAX_SIZE); if (!port0) D_GOTO(unlock, rc = -DER_NOMEM); } if (auth_key) { - D_STRNDUP(auth_key0, auth_key, 255); + D_STRNDUP(auth_key0, auth_key, CRT_ENV_STR_MAX_SIZE); if (!auth_key0) D_GOTO(unlock, rc = -DER_NOMEM); } @@ -1190,19 +1193,19 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char } if (interface) { - D_STRNDUP(na_cfg->noc_interface, interface, 64); + D_STRNDUP(na_cfg->noc_interface, interface, CRT_ENV_STR_MAX_SIZE); if (!na_cfg->noc_interface) D_GOTO(out, rc = -DER_NOMEM); } if (domain) { - D_STRNDUP(na_cfg->noc_domain, domain, 64); + D_STRNDUP(na_cfg->noc_domain, domain, CRT_ENV_STR_MAX_SIZE); if (!na_cfg->noc_domain) D_GOTO(out, rc = -DER_NOMEM); } if (auth_key) { - D_STRNDUP(na_cfg->noc_auth_key, auth_key, 255); + D_STRNDUP(na_cfg->noc_auth_key, auth_key, CRT_ENV_STR_MAX_SIZE); if (!na_cfg->noc_auth_key) D_GOTO(out, rc = -DER_NOMEM); } @@ -1212,12 +1215,22 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char count = 1; save_ptr = na_cfg->noc_interface; - while (*save_ptr != '\0') { + while (*save_ptr != '\0' && count < CRT_ENV_STR_MAX_SIZE) { if (*save_ptr == ',') count++; save_ptr++; } + /* + * env checks limit strings to CRT_ENV_STR_MAX_SIZE, but an interface can + * be passed as an init argument + */ + if (count >= CRT_ENV_STR_MAX_SIZE) { + D_ERROR("Interface value '%s' exceeds limit of %d characters\n", + na_cfg->noc_interface, CRT_ENV_STR_MAX_SIZE); + D_GOTO(out, rc = -DER_INVAL); + } + D_ALLOC_ARRAY(na_cfg->noc_iface_str, count); if (!na_cfg->noc_iface_str) D_GOTO(out, rc = -DER_NOMEM); @@ -1227,6 +1240,7 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char idx = 0; token = strtok_r(na_cfg->noc_interface, ",", &save_ptr); while (token != NULL) { + /* TODO: If needed add filtering for duplicate interfaces here */ na_cfg->noc_iface_str[idx] = token; token = strtok_r(NULL, ",", &save_ptr); idx++; @@ -1242,12 +1256,22 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char count = 1; save_ptr = na_cfg->noc_domain; - while (*save_ptr != '\0') { + while (*save_ptr != '\0' && count < CRT_ENV_STR_MAX_SIZE) { if (*save_ptr == ',') count++; save_ptr++; } + /* + * env checks limit strings to CRT_ENV_STR_MAX_SIZE, but a domain can + * be passed as an init argument + */ + if (count >= CRT_ENV_STR_MAX_SIZE) { + D_ERROR("Domain value '%s' exceeds limit of %d characters\n", + na_cfg->noc_domain, CRT_ENV_STR_MAX_SIZE); + D_GOTO(out, rc = -DER_INVAL); + } + D_ALLOC_ARRAY(na_cfg->noc_domain_str, count); if (!na_cfg->noc_domain_str) D_GOTO(out, rc = -DER_NOMEM); @@ -1267,8 +1291,9 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char na_cfg->noc_domain_total = count; if (na_cfg->noc_domain_total > 0 && na_cfg->noc_domain_total != na_cfg->noc_iface_total) { - D_ERROR("Mismatched number of domains (%d) and interfaces (%d) specified\n", - na_cfg->noc_domain_total, na_cfg->noc_iface_total); + D_ERROR("Mismatched # of domains [%d]='%s' and interfaces [%d]='%s' specified\n", + na_cfg->noc_domain_total, na_cfg->noc_domain, na_cfg->noc_iface_total, + na_cfg->noc_interface); D_GOTO(out, rc = -DER_INVAL); } diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 472d266fb06..99bc291c2b8 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -1,7 +1,7 @@ /* * (C) Copyright 2016-2024 Intel Corporation. * (C) Copyright 2025 Google LLC - * (C) Copyright 2025 Hewlett Packard Enterprise Development LP + * (C) Copyright 2025-2026 Hewlett Packard Enterprise Development LP * * SPDX-License-Identifier: BSD-2-Clause-Patent */ @@ -190,6 +190,8 @@ struct crt_event_cb_priv { #define CRT_CALLBACKS_NUM (4) /* start number of CBs */ #endif +#define CRT_ENV_STR_MAX_SIZE 1024 + /* * List of environment variables to read at CaRT library load time. * for integer envs use ENV() @@ -251,13 +253,13 @@ struct crt_event_cb_priv { #define ENV(x) \ unsigned int _##x; \ int _rc_##x; \ - int _no_print_##x; + bool _no_print_##x; /* char* env */ #define ENV_STR(x) \ char *_##x; \ int _rc_##x; \ - int _no_print_##x; + bool _no_print_##x; #define ENV_STR_NO_PRINT(x) ENV_STR(x) @@ -275,6 +277,7 @@ extern struct crt_envs crt_genvs; static inline void crt_env_fini(void); +/* init cart env structure */ static inline void crt_env_init(void) { @@ -285,19 +288,19 @@ crt_env_init(void) #define ENV(x) \ do { \ crt_genvs._rc_##x = d_getenv_uint(#x, &crt_genvs._##x); \ - crt_genvs._no_print_##x = 0; \ + crt_genvs._no_print_##x = false; \ } while (0); #define ENV_STR(x) \ do { \ crt_genvs._rc_##x = d_agetenv_str(&crt_genvs._##x, #x); \ - crt_genvs._no_print_##x = 0; \ + crt_genvs._no_print_##x = false; \ } while (0); #define ENV_STR_NO_PRINT(x) \ do { \ crt_genvs._rc_##x = d_agetenv_str(&crt_genvs._##x, #x); \ - crt_genvs._no_print_##x = 1; \ + crt_genvs._no_print_##x = true; \ } while (0); CRT_ENV_LIST; @@ -308,6 +311,7 @@ crt_env_init(void) crt_genvs.inited = true; } +/* fini cart envs */ static inline void crt_env_fini(void) { @@ -324,7 +328,7 @@ crt_env_fini(void) crt_genvs.inited = false; } -/* Returns value if env was present at load time */ +/* Returns value if env was present at load time and is part of CRT_ENV_LIST */ #define crt_env_get(name, val) \ do { \ D_ASSERT(crt_genvs.inited); \ @@ -332,6 +336,38 @@ crt_env_fini(void) *val = crt_genvs._##name; \ } while (0) +/* Check envs that contain strings to not exceed CRT_ENV_STR_MAX_SIZE */ +static inline bool +crt_env_list_valid(void) +{ +/* Ignore non-string envs in this check */ +#define ENV(x) + +/* if string env exceeds CRT_ENV_STR_MAX_SIZE - return false */ +#define ENV_STR(x) \ + if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) > CRT_ENV_STR_MAX_SIZE) { \ + D_ERROR("env '%s' (value='%s') exceeded max size %d\n", #x, crt_genvs._##x, \ + CRT_ENV_STR_MAX_SIZE); \ + return false; \ + } + +/* if string env exceeds CRT_ENV_STR_MAX_SIZE - return false */ +#define ENV_STR_NO_PRINT(x) \ + if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) > CRT_ENV_STR_MAX_SIZE) { \ + D_ERROR("env '%s' exceeded max size %d\n", #x, CRT_ENV_STR_MAX_SIZE); \ + return false; \ + } + + /* expand env list using the above ENV_* definitions */ + CRT_ENV_LIST; + return true; + +#undef ENV +#undef ENV_STR +#undef ENV_STR_NO_PRINT +} + +/* dump environment variables from the CRT_ENV_LIST */ static inline void crt_env_dump(void) { From 19cb85863e0795055893fc79cff6089fe24a54e9 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Wed, 28 Jan 2026 17:07:32 +0000 Subject: [PATCH 2/3] - off by 1 Signed-off-by: Alexander A Oganezov --- src/cart/crt_internal_types.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cart/crt_internal_types.h b/src/cart/crt_internal_types.h index 99bc291c2b8..9690213737b 100644 --- a/src/cart/crt_internal_types.h +++ b/src/cart/crt_internal_types.h @@ -345,7 +345,7 @@ crt_env_list_valid(void) /* if string env exceeds CRT_ENV_STR_MAX_SIZE - return false */ #define ENV_STR(x) \ - if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) > CRT_ENV_STR_MAX_SIZE) { \ + if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) + 1 > CRT_ENV_STR_MAX_SIZE) { \ D_ERROR("env '%s' (value='%s') exceeded max size %d\n", #x, crt_genvs._##x, \ CRT_ENV_STR_MAX_SIZE); \ return false; \ @@ -353,7 +353,7 @@ crt_env_list_valid(void) /* if string env exceeds CRT_ENV_STR_MAX_SIZE - return false */ #define ENV_STR_NO_PRINT(x) \ - if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) > CRT_ENV_STR_MAX_SIZE) { \ + if (crt_genvs._rc_##x == 0 && strlen(crt_genvs._##x) + 1 > CRT_ENV_STR_MAX_SIZE) { \ D_ERROR("env '%s' exceeded max size %d\n", #x, CRT_ENV_STR_MAX_SIZE); \ return false; \ } From 47fa6e565b11d25426e01befca0425bd8e2ed015 Mon Sep 17 00:00:00 2001 From: Alexander A Oganezov Date: Wed, 28 Jan 2026 18:20:51 +0000 Subject: [PATCH 3/3] - fix Signed-off-by: Alexander A Oganezov --- src/cart/crt_init.c | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/cart/crt_init.c b/src/cart/crt_init.c index ace5b3ba479..e4556b8693d 100644 --- a/src/cart/crt_init.c +++ b/src/cart/crt_init.c @@ -1211,26 +1211,26 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char } if (na_cfg->noc_interface) { - /* count number of ','-separated interfaces */ - count = 1; - save_ptr = na_cfg->noc_interface; - - while (*save_ptr != '\0' && count < CRT_ENV_STR_MAX_SIZE) { - if (*save_ptr == ',') - count++; - save_ptr++; - } - /* * env checks limit strings to CRT_ENV_STR_MAX_SIZE, but an interface can * be passed as an init argument */ - if (count >= CRT_ENV_STR_MAX_SIZE) { + if (strlen(na_cfg->noc_interface) + 1 >= CRT_ENV_STR_MAX_SIZE) { D_ERROR("Interface value '%s' exceeds limit of %d characters\n", na_cfg->noc_interface, CRT_ENV_STR_MAX_SIZE); D_GOTO(out, rc = -DER_INVAL); } + /* count number of ','-separated interfaces */ + count = 1; + save_ptr = na_cfg->noc_interface; + + while (*save_ptr != '\0') { + if (*save_ptr == ',') + count++; + save_ptr++; + } + D_ALLOC_ARRAY(na_cfg->noc_iface_str, count); if (!na_cfg->noc_iface_str) D_GOTO(out, rc = -DER_NOMEM); @@ -1252,26 +1252,26 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char count = 0; if (na_cfg->noc_domain) { - /* count number of ','-separated domains */ - count = 1; - save_ptr = na_cfg->noc_domain; - - while (*save_ptr != '\0' && count < CRT_ENV_STR_MAX_SIZE) { - if (*save_ptr == ',') - count++; - save_ptr++; - } - /* * env checks limit strings to CRT_ENV_STR_MAX_SIZE, but a domain can * be passed as an init argument */ - if (count >= CRT_ENV_STR_MAX_SIZE) { + if (strlen(na_cfg->noc_domain) + 1 >= CRT_ENV_STR_MAX_SIZE) { D_ERROR("Domain value '%s' exceeds limit of %d characters\n", na_cfg->noc_domain, CRT_ENV_STR_MAX_SIZE); D_GOTO(out, rc = -DER_INVAL); } + /* count number of ','-separated domains */ + count = 1; + save_ptr = na_cfg->noc_domain; + + while (*save_ptr != '\0') { + if (*save_ptr == ',') + count++; + save_ptr++; + } + D_ALLOC_ARRAY(na_cfg->noc_domain_str, count); if (!na_cfg->noc_domain_str) D_GOTO(out, rc = -DER_NOMEM);