Skip to content

DAOS-18527 cart: Handling of string env limits #17466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mchaarawi merged 3 commits into from
Feb 3, 2026
Merged

DAOS-18527 cart: Handling of string env limits #17466

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f5d7105
DAOS-18527 cart: Handling of string env limits
frostedcmos Jan 27, 2026
19cb858
- off by 1
frostedcmos Jan 28, 2026
47fa6e5
- fix
frostedcmos Jan 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 36 additions & 11 deletions src/cart/crt_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -1190,24 +1193,34 @@ 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);
Copy link
Contributor

@mchaarawi mchaarawi Feb 2, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we try to make this more robust?
this is not necessarily an env variable, and sometimes it can contain multiple interfaces like the issue Johann had, so like CRT_ENV_STR_MAX_SIZE * number of interfaces (seprated be , or : forgot what it is here)

Copy link
Contributor

@mchaarawi mchaarawi Feb 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im not opposed to this as is.. i think 1k is probably enough to cover 60 interfaces/domains or more :-)

Copy link
Contributor Author

@frostedcmos frostedcmos Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am mixed on this. On one hand, i agree that having individual limits would be better, but then we would have to do similar for other strings as well such as domains, provider and ports (for multiprovider case) and possibly others (like auth keys).

If we change this to be CRT_ENV_STR_MAX_SIZE * num_interfaces here, then we would have to allow the similar sized string to be passed via D_INTERFACE environment variable as well. This would require changing each cart env declaration to specify an individual limit.

In addition, we dont know the number of interfaces ahead of time, and we only access envs once they are all copied out by cart (at init time), so even at best it would need to be some static amount such as CRT_ENV_STR_MAX_SIZE * max_num_interfaces for D_INTERFACE and CRT_ENV_STR_MAX_SIZE * max_num_providers for D_PROVIDER, etc..

With all that i feel that having a single max value big enough for all input strings is much cleaner for now.

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);
}

if (na_cfg->noc_interface) {
/*
* env checks limit strings to CRT_ENV_STR_MAX_SIZE, but an interface can
* be passed as an init argument
*/
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;
Expand All @@ -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++;
Expand All @@ -1238,6 +1252,16 @@ crt_na_config_init(bool primary, crt_provider_t provider, char *interface, char

count = 0;
if (na_cfg->noc_domain) {
/*
* env checks limit strings to CRT_ENV_STR_MAX_SIZE, but a domain can
* be passed as an init argument
*/
if (strlen(na_cfg->noc_domain) + 1 >= CRT_ENV_STR_MAX_SIZE) {
Copy link
Collaborator

@soumagne soumagne Jan 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need parentheses here ? I'm surprised the compiler does not complain if it does not.

Copy link
Contributor Author

@frostedcmos frostedcmos Jan 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why ? math operations take precedence over comparisons

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;
Expand Down Expand Up @@ -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);
}

Expand Down
50 changes: 43 additions & 7 deletions src/cart/crt_internal_types.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
*/
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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)

Expand All @@ -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)
{
Expand All @@ -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;
Expand All @@ -308,6 +311,7 @@ crt_env_init(void)
crt_genvs.inited = true;
}

/* fini cart envs */
static inline void
crt_env_fini(void)
{
Expand All @@ -324,14 +328,46 @@ 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); \
if (crt_genvs._rc_##name == 0) \
*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) + 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; \
}

/* 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) + 1 > 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)
{
Expand Down
Loading