Skip to content

Commit

Permalink
spa_prop_get: require caller to supply output nvlist
Browse files Browse the repository at this point in the history 
All callers to spa_prop_get() and spa_prop_get_nvlist() supplied their
own preallocated nvlist (except ztest), so we can remove the option to
have them allocate one if none is supplied.

This sidesteps a bug in spa_prop_get(), where the error var wasn't
initialised, which could lead to the provided nvlist being freed at the
end.

Sponsored-by: Klara, Inc.
Sponsored-by: Wasabi Technology, Inc.
Signed-off-by: Rob Norris <[email protected]>
  • Loading branch information
@robn
robn committed Sep 4, 2024
1 parent bf8c61f commit 17031aa
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 33 deletions.
5 changes: 3 additions & 2 deletions cmd/ztest.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6211,13 +6211,14 @@ void
ztest_spa_prop_get_set(ztest_ds_t *zd, uint64_t id)
{
(void) zd, (void) id;
nvlist_t *props = NULL;

(void) pthread_rwlock_rdlock(&ztest_name_lock);

(void) ztest_spa_prop_set_uint64(ZPOOL_PROP_AUTOTRIM, ztest_random(2));

VERIFY0(spa_prop_get(ztest_spa, &props));
nvlist_t *props = fnvlist_alloc();

VERIFY0(spa_prop_get(ztest_spa, props));

if (ztest_opts.zo_verbose >= 6)
dump_nvlist(props, 4);
Expand Down
4 changes: 2 additions & 2 deletions include/sys/spa.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1201,9 +1201,9 @@ extern void spa_boot_init(void);

/* properties */
extern int spa_prop_set(spa_t *spa, nvlist_t *nvp);
extern int spa_prop_get(spa_t *spa, nvlist_t **nvp);
extern int spa_prop_get(spa_t *spa, nvlist_t *nvp);
extern int spa_prop_get_nvlist(spa_t *spa, char **props,
unsigned int n_props, nvlist_t **outnvl);
unsigned int n_props, nvlist_t *outnvl);
extern void spa_prop_clear_bootfs(spa_t *spa, uint64_t obj, dmu_tx_t *tx);
extern void spa_configfile_set(spa_t *, nvlist_t *, boolean_t);

Expand Down
34 changes: 10 additions & 24 deletions module/zfs/spa.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -366,21 +366,15 @@ spa_prop_add(spa_t *spa, const char *propname, nvlist_t *outnvl)

int
spa_prop_get_nvlist(spa_t *spa, char **props, unsigned int n_props,
nvlist_t **outnvl)
nvlist_t *outnvl)
{
int err = 0;

if (props == NULL)
return (0);

if (*outnvl == NULL) {
err = nvlist_alloc(outnvl, NV_UNIQUE_NAME, KM_SLEEP);
if (err)
return (err);
}

for (unsigned int i = 0; i < n_props && err == 0; i++) {
err = spa_prop_add(spa, props[i], *outnvl);
err = spa_prop_add(spa, props[i], outnvl);
}

return (err);
Expand Down Expand Up @@ -544,19 +538,13 @@ spa_prop_get_config(spa_t *spa, nvlist_t **nvp)
* Get zpool property values.
*/
int
spa_prop_get(spa_t *spa, nvlist_t **nvp)
spa_prop_get(spa_t *spa, nvlist_t *nv)
{
objset_t *mos = spa->spa_meta_objset;
zap_cursor_t zc;
zap_attribute_t za;
dsl_pool_t *dp;
int err;

if (*nvp == NULL) {
err = nvlist_alloc(nvp, NV_UNIQUE_NAME, KM_SLEEP);
if (err)
return (err);
}
int err = 0;

dp = spa_get_dsl(spa);
dsl_pool_config_enter(dp, FTAG);
Expand All @@ -565,7 +553,7 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
/*
* Get properties from the spa config.
*/
spa_prop_get_config(spa, nvp);
spa_prop_get_config(spa, &nv);

/* If no pool property object, no more prop to get. */
if (mos == NULL || spa->spa_pool_props_object == 0)
Expand Down Expand Up @@ -610,7 +598,7 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
intval = za.za_first_integer;
}

spa_prop_add_list(*nvp, prop, strval, intval, src);
spa_prop_add_list(nv, prop, strval, intval, src);

if (strval != NULL)
kmem_free(strval, ZFS_MAX_DATASET_NAME_LEN);
Expand All @@ -627,10 +615,10 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
break;
}
if (prop != ZPOOL_PROP_INVAL) {
spa_prop_add_list(*nvp, prop, strval, 0, src);
spa_prop_add_list(nv, prop, strval, 0, src);
} else {
src = ZPROP_SRC_LOCAL;
spa_prop_add_user(*nvp, za.za_name, strval,
spa_prop_add_user(nv, za.za_name, strval,
src);
}
kmem_free(strval, za.za_num_integers);
Expand All @@ -644,11 +632,9 @@ spa_prop_get(spa_t *spa, nvlist_t **nvp)
out:
mutex_exit(&spa->spa_props_lock);
dsl_pool_config_exit(dp, FTAG);
if (err && err != ENOENT) {
nvlist_free(*nvp);
*nvp = NULL;

if (err && err != ENOENT)
return (err);
}

return (0);
}
Expand Down
10 changes: 5 additions & 5 deletions module/zfs/zfs_ioctl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3050,7 +3050,6 @@ static const zfs_ioc_key_t zfs_keys_get_props[] = {
static int
zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
{
nvlist_t *nvp = outnvl;
spa_t *spa;
char **props = NULL;
unsigned int n_props = 0;
Expand All @@ -3069,16 +3068,17 @@ zfs_ioc_pool_get_props(const char *pool, nvlist_t *innvl, nvlist_t *outnvl)
*/
mutex_enter(&spa_namespace_lock);
if ((spa = spa_lookup(pool)) != NULL) {
error = spa_prop_get(spa, &nvp);
error = spa_prop_get(spa, outnvl);
if (error == 0 && props != NULL)
error = spa_prop_get_nvlist(spa, props, n_props,
&nvp);
outnvl);
}
mutex_exit(&spa_namespace_lock);
} else {
error = spa_prop_get(spa, &nvp);
error = spa_prop_get(spa, outnvl);
if (error == 0 && props != NULL)
error = spa_prop_get_nvlist(spa, props, n_props, &nvp);
error = spa_prop_get_nvlist(spa, props, n_props,
outnvl);
spa_close(spa, FTAG);
}

Expand Down

0 comments on commit 17031aa

Please sign in to comment.