From ea77d0c04a38b620f93d2b4a6d03f817a6acc3c9 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Mon, 26 Sep 2022 20:18:05 -0400 Subject: [PATCH] Fix userspace memory leaks found by Clang Static Analzyer Recently, I have been making a push to fix things that coverity found. However, I was curious what Clang's static analyzer reported, so I ran it and found things that coverity had missed. * contrib/pam_zfs_key/pam_zfs_key.c: If prop_mountpoint is passed more than once, we leak memory. * module/zfs/zcp_get.c: We leak memory on temporary properties in userspace. * tests/zfs-tests/cmd/draid.c: On error from vdev_draid_rand(), we leak memory if best_map had been allocated by a prior iteration. * tests/zfs-tests/cmd/mkfile.c: Memory used by the loop is not freed before program termination. Arguably, these are all minor issues, but if we ignore them, then they could obscure serious bugs, so we fix them. Reviewed-by: Brian Behlendorf Signed-off-by: Richard Yao Closes #13955 --- contrib/pam_zfs_key/pam_zfs_key.c | 3 ++- module/zfs/zcp_get.c | 1 + tests/zfs-tests/cmd/draid.c | 5 ++++- tests/zfs-tests/cmd/mkfile.c | 4 ++++ 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/contrib/pam_zfs_key/pam_zfs_key.c b/contrib/pam_zfs_key/pam_zfs_key.c index c1001e6b81c2..aaca670080b8 100644 --- a/contrib/pam_zfs_key/pam_zfs_key.c +++ b/contrib/pam_zfs_key/pam_zfs_key.c @@ -480,7 +480,8 @@ zfs_key_config_load(pam_handle_t *pamh, zfs_key_config_t *config, } else if (strcmp(argv[c], "nounmount") == 0) { config->unmount_and_unload = 0; } else if (strcmp(argv[c], "prop_mountpoint") == 0) { - config->homedir = strdup(entry->pw_dir); + if (config->homedir == NULL) + config->homedir = strdup(entry->pw_dir); } } return (0); diff --git a/module/zfs/zcp_get.c b/module/zfs/zcp_get.c index 0a0466d46969..8230a4193662 100644 --- a/module/zfs/zcp_get.c +++ b/module/zfs/zcp_get.c @@ -472,6 +472,7 @@ get_zap_prop(lua_State *state, dsl_dataset_t *ds, zfs_prop_t zfs_prop) /* Fill in temporary value for prop, if applicable */ (void) zfs_get_temporary_prop(ds, zfs_prop, &numval, setpoint); #else + kmem_free(strval, ZAP_MAXVALUELEN); return (luaL_error(state, "temporary properties only supported in kernel mode", prop_name)); diff --git a/tests/zfs-tests/cmd/draid.c b/tests/zfs-tests/cmd/draid.c index 869ca902d083..39b58a709cec 100644 --- a/tests/zfs-tests/cmd/draid.c +++ b/tests/zfs-tests/cmd/draid.c @@ -720,8 +720,11 @@ eval_maps(uint64_t children, int passes, uint64_t *map_seed, */ error = alloc_new_map(children, MAP_ROWS_DEFAULT, vdev_draid_rand(map_seed), &map); - if (error) + if (error) { + if (best_map != NULL) + free_map(best_map); return (error); + } /* * Consider maps with a lower worst_ratio to be of higher diff --git a/tests/zfs-tests/cmd/mkfile.c b/tests/zfs-tests/cmd/mkfile.c index 7ce50e6a37c4..3b61deed6bf5 100644 --- a/tests/zfs-tests/cmd/mkfile.c +++ b/tests/zfs-tests/cmd/mkfile.c @@ -276,5 +276,9 @@ main(int argc, char **argv) argv++; argc--; } + + if (buf) + free(buf); + return (errors); }