From f7bda2de9786017348fd814603735f2be4182825 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 c1001e6b81..aaca670080 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 0a0466d469..8230a41936 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 869ca902d0..39b58a709c 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 7ce50e6a37..3b61deed6b 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); }