From edc508ac0b80996d872f3c5d86c2ee49b519a661 Mon Sep 17 00:00:00 2001 From: Christian Schwarz Date: Tue, 16 Feb 2021 12:27:48 +0100 Subject: [PATCH] libzpool: set_global_var: refactor to not modify 'arg' Also fixes leak of the dlopen handle in the error case. Reviewed-by: Matthew Ahrens Reviewed-by: Brian Behlendorf Reviewed-by: Pavel Zakharov Signed-off-by: Christian Schwarz Closes #11602 --- include/sys/zfs_context.h | 2 +- lib/libzpool/util.c | 72 +++++++++++++++++++++++++++++---------- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/include/sys/zfs_context.h b/include/sys/zfs_context.h index ee3216d67..2ec576f0e 100644 --- a/include/sys/zfs_context.h +++ b/include/sys/zfs_context.h @@ -652,7 +652,7 @@ extern void random_fini(void); struct spa; extern void show_pool_stats(struct spa *); -extern int set_global_var(char *arg); +extern int set_global_var(char const *arg); typedef struct callb_cpr { kmutex_t *cc_lockp; diff --git a/lib/libzpool/util.c b/lib/libzpool/util.c index 13d585299..2da2375a1 100644 --- a/lib/libzpool/util.c +++ b/lib/libzpool/util.c @@ -148,16 +148,52 @@ show_pool_stats(spa_t *spa) nvlist_free(config); } +/* *k_out must be freed by the caller */ +static int +set_global_var_parse_kv(const char *arg, char **k_out, u_longlong_t *v_out) +{ + int err; + VERIFY(arg); + char *d = strdup(arg); + + char *save = NULL; + char *k = strtok_r(d, "=", &save); + char *v_str = strtok_r(NULL, "=", &save); + char *follow = strtok_r(NULL, "=", &save); + if (k == NULL || v_str == NULL || follow != NULL) { + err = EINVAL; + goto err_free; + } + + u_longlong_t val = strtoull(v_str, NULL, 0); + if (val > UINT32_MAX) { + fprintf(stderr, "Value for global variable '%s' must " + "be a 32-bit unsigned integer, got '%s'\n", k, v_str); + err = EOVERFLOW; + goto err_free; + } + + *k_out = k; + *v_out = val; + return (0); + +err_free: + free(k); + + return (err); +} + /* * Sets given global variable in libzpool to given unsigned 32-bit value. * arg: "=" */ int -set_global_var(char *arg) +set_global_var(char const *arg) { void *zpoolhdl; - char *varname = arg, *varval; + char *varname; u_longlong_t val; + int ret; #ifndef _ZFS_LITTLE_ENDIAN /* @@ -167,19 +203,12 @@ set_global_var(char *arg) */ fprintf(stderr, "Setting global variables is only supported on " "little-endian systems\n"); - return (ENOTSUP); + ret = ENOTSUP; + goto out_ret; #endif - if (arg != NULL && (varval = strchr(arg, '=')) != NULL) { - *varval = '\0'; - varval++; - val = strtoull(varval, NULL, 0); - if (val > UINT32_MAX) { - fprintf(stderr, "Value for global variable '%s' must " - "be a 32-bit unsigned integer\n", varname); - return (EOVERFLOW); - } - } else { - return (EINVAL); + + if ((ret = set_global_var_parse_kv(arg, &varname, &val)) != 0) { + goto out_ret; } zpoolhdl = dlopen("libzpool.so", RTLD_LAZY); @@ -189,18 +218,25 @@ set_global_var(char *arg) if (var == NULL) { fprintf(stderr, "Global variable '%s' does not exist " "in libzpool.so\n", varname); - return (EINVAL); + ret = EINVAL; + goto out_dlclose; } *var = (uint32_t)val; - dlclose(zpoolhdl); } else { fprintf(stderr, "Failed to open libzpool.so to set global " "variable\n"); - return (EIO); + ret = EIO; + goto out_dlclose; } - return (0); + ret = 0; + +out_dlclose: + dlclose(zpoolhdl); + free(varname); +out_ret: + return (ret); } static nvlist_t *