From 72c99dc95961c1a91a1504173fd447f92f73ad50 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Thu, 6 Oct 2022 20:18:40 -0400 Subject: [PATCH] Handle possible null pointers from malloc/strdup/strndup() GCC 12.1.1_p20220625's static analyzer caught these. Of the two in the btree test, one had previously been caught by Coverity and Smatch, but GCC flagged it as a false positive. Upon examining how other test cases handle this, the solution was changed from `ASSERT3P(node, !=, NULL);` to using `perror()` to be consistent with the fixes to the other fixes done to the ZTS code. That approach was also used in ZED since I did not see a better way of handling this there. Also, upon inspection, additional unchecked pointers from malloc()/calloc()/strdup() were found in ZED, so those were handled too. In other parts of the code, the existing methods to avoid issues from memory allocators returning NULL were used, such as using `umem_alloc(size, UMEM_NOFAIL)` or returning `ENOMEM`. Reviewed-by: Brian Behlendorf Reviewed-by: Tony Hutter Signed-off-by: Richard Yao Closes #13979 --- cmd/zed/agents/fmd_serd.c | 19 +++++++++++++++++++ cmd/zed/agents/zfs_mod.c | 10 ++++++++++ cmd/zed/zed_exec.c | 4 ++++ cmd/ztest.c | 9 +++++---- lib/libzfs/libzfs_sendrecv.c | 5 +++-- lib/libzutil/zutil_import.c | 3 +++ tests/zfs-tests/cmd/badsend.c | 4 ++++ tests/zfs-tests/cmd/btree_test.c | 9 ++++++++- tests/zfs-tests/cmd/file/largest_file.c | 2 ++ tests/zfs-tests/cmd/nvlist_to_lua.c | 5 +++++ 10 files changed, 63 insertions(+), 7 deletions(-) diff --git a/cmd/zed/agents/fmd_serd.c b/cmd/zed/agents/fmd_serd.c index 763ecb958..0bb2c535f 100644 --- a/cmd/zed/agents/fmd_serd.c +++ b/cmd/zed/agents/fmd_serd.c @@ -74,9 +74,18 @@ fmd_serd_eng_alloc(const char *name, uint64_t n, hrtime_t t) fmd_serd_eng_t *sgp; sgp = malloc(sizeof (fmd_serd_eng_t)); + if (sgp == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } memset(sgp, 0, sizeof (fmd_serd_eng_t)); sgp->sg_name = strdup(name); + if (sgp->sg_name == NULL) { + perror("strdup"); + exit(EXIT_FAILURE); + } + sgp->sg_flags = FMD_SERD_DIRTY; sgp->sg_n = n; sgp->sg_t = t; @@ -123,6 +132,12 @@ fmd_serd_hash_create(fmd_serd_hash_t *shp) shp->sh_hashlen = FMD_STR_BUCKETS; shp->sh_hash = calloc(shp->sh_hashlen, sizeof (void *)); shp->sh_count = 0; + + if (shp->sh_hash == NULL) { + perror("calloc"); + exit(EXIT_FAILURE); + } + } void @@ -241,6 +256,10 @@ fmd_serd_eng_record(fmd_serd_eng_t *sgp, hrtime_t hrt) fmd_serd_eng_discard(sgp, list_tail(&sgp->sg_list)); sep = malloc(sizeof (fmd_serd_elem_t)); + if (sep == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } sep->se_hrt = hrt; list_insert_head(&sgp->sg_list, sep); diff --git a/cmd/zed/agents/zfs_mod.c b/cmd/zed/agents/zfs_mod.c index 53d9ababd..e149c27ee 100644 --- a/cmd/zed/agents/zfs_mod.c +++ b/cmd/zed/agents/zfs_mod.c @@ -133,6 +133,11 @@ zfs_unavail_pool(zpool_handle_t *zhp, void *data) if (zfs_toplevel_state(zhp) < VDEV_STATE_DEGRADED) { unavailpool_t *uap; uap = malloc(sizeof (unavailpool_t)); + if (uap == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } + uap->uap_zhp = zhp; list_insert_tail((list_t *)data, uap); } else { @@ -411,6 +416,11 @@ zfs_process_add(zpool_handle_t *zhp, nvlist_t *vdev, boolean_t labeled) * completed. */ device = malloc(sizeof (pendingdev_t)); + if (device == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } + (void) strlcpy(device->pd_physpath, physpath, sizeof (device->pd_physpath)); list_insert_tail(&g_device_list, device); diff --git a/cmd/zed/zed_exec.c b/cmd/zed/zed_exec.c index 51c292d41..e45acfb2c 100644 --- a/cmd/zed/zed_exec.c +++ b/cmd/zed/zed_exec.c @@ -175,6 +175,10 @@ _zed_exec_fork_child(uint64_t eid, const char *dir, const char *prog, node->pid = pid; node->eid = eid; node->name = strdup(prog); + if (node->name == NULL) { + perror("strdup"); + exit(EXIT_FAILURE); + } avl_add(&_launched_processes, node); } diff --git a/cmd/ztest.c b/cmd/ztest.c index 668a267b6..25f12f967 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -3512,7 +3512,8 @@ ztest_split_pool(ztest_ds_t *zd, uint64_t id) VERIFY0(nvlist_lookup_nvlist_array(tree, ZPOOL_CONFIG_CHILDREN, &child, &children)); - schild = malloc(rvd->vdev_children * sizeof (nvlist_t *)); + schild = umem_alloc(rvd->vdev_children * sizeof (nvlist_t *), + UMEM_NOFAIL); for (c = 0; c < children; c++) { vdev_t *tvd = rvd->vdev_child[c]; nvlist_t **mchild; @@ -3546,7 +3547,7 @@ ztest_split_pool(ztest_ds_t *zd, uint64_t id) for (c = 0; c < schildren; c++) fnvlist_free(schild[c]); - free(schild); + umem_free(schild, rvd->vdev_children * sizeof (nvlist_t *)); fnvlist_free(split); spa_config_exit(spa, SCL_VDEV, FTAG); @@ -6663,7 +6664,7 @@ join_strings(char **strings, const char *sep) } size_t buflen = totallen + 1; - char *o = malloc(buflen); /* trailing 0 byte */ + char *o = umem_alloc(buflen, UMEM_NOFAIL); /* trailing 0 byte */ o[0] = '\0'; for (char **sp = strings; *sp != NULL; sp++) { size_t would; @@ -6925,7 +6926,7 @@ ztest_run_zdb(const char *pool) pool); ASSERT3U(would, <, len); - free(set_gvars_args_joined); + umem_free(set_gvars_args_joined, strlen(set_gvars_args_joined) + 1); if (ztest_opts.zo_verbose >= 5) (void) printf("Executing %s\n", zdb); diff --git a/lib/libzfs/libzfs_sendrecv.c b/lib/libzfs/libzfs_sendrecv.c index d63a9e1a4..bf93ac9ba 100644 --- a/lib/libzfs/libzfs_sendrecv.c +++ b/lib/libzfs/libzfs_sendrecv.c @@ -4387,7 +4387,7 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, * prepend a path separator. */ int len = strlen(drrb->drr_toname); - cp = malloc(len + 2); + cp = umem_alloc(len + 2, UMEM_NOFAIL); cp[0] = '/'; (void) strcpy(&cp[1], drrb->drr_toname); chopprefix = cp; @@ -4440,7 +4440,8 @@ zfs_receive_one(libzfs_handle_t *hdl, int infd, const char *tosnap, */ (void) strlcpy(destsnap, tosnap, sizeof (destsnap)); (void) strlcat(destsnap, chopprefix, sizeof (destsnap)); - free(cp); + if (cp != NULL) + umem_free(cp, strlen(cp) + 1); if (!zfs_name_valid(destsnap, ZFS_TYPE_SNAPSHOT)) { err = zfs_error(hdl, EZFS_INVALIDNAME, errbuf); goto out; diff --git a/lib/libzutil/zutil_import.c b/lib/libzutil/zutil_import.c index fbf390587..e7a755dcb 100644 --- a/lib/libzutil/zutil_import.c +++ b/lib/libzutil/zutil_import.c @@ -1845,6 +1845,9 @@ zpool_find_config(libpc_handle_t *hdl, const char *target, nvlist_t **configp, int count = 0; char *targetdup = strdup(target); + if (targetdup == NULL) + return (ENOMEM); + *configp = NULL; if ((sepp = strpbrk(targetdup, "/@")) != NULL) diff --git a/tests/zfs-tests/cmd/badsend.c b/tests/zfs-tests/cmd/badsend.c index 48d05bd3f..7a54532fb 100644 --- a/tests/zfs-tests/cmd/badsend.c +++ b/tests/zfs-tests/cmd/badsend.c @@ -76,6 +76,10 @@ main(int argc, char const * const argv[]) tosnap = p + 1; fsname = strndup(tofull, p - tofull); + if (fsname == NULL) { + perror("strndup"); + exit(EXIT_FAILURE); + } if (strncmp(fsname, fromfull, p - tofull) != 0) usage(argv[0]); diff --git a/tests/zfs-tests/cmd/btree_test.c b/tests/zfs-tests/cmd/btree_test.c index 4e2023003..ab8967b22 100644 --- a/tests/zfs-tests/cmd/btree_test.c +++ b/tests/zfs-tests/cmd/btree_test.c @@ -242,7 +242,10 @@ drain_tree(zfs_btree_t *bt, char *why) zfs_btree_add_idx(bt, &randval, &bt_idx); node = malloc(sizeof (int_node_t)); - ASSERT3P(node, !=, NULL); + if (node == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } node->data = randval; if ((ret = avl_find(&avl, node, &avl_idx)) != NULL) { @@ -319,6 +322,10 @@ stress_tree(zfs_btree_t *bt, char *why) uint64_t randval = random() % tree_limit; node = malloc(sizeof (*node)); + if (node == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } node->data = randval; max = randval > max ? randval : max; diff --git a/tests/zfs-tests/cmd/file/largest_file.c b/tests/zfs-tests/cmd/file/largest_file.c index 8545bb7ee..d7252556b 100644 --- a/tests/zfs-tests/cmd/file/largest_file.c +++ b/tests/zfs-tests/cmd/file/largest_file.c @@ -78,6 +78,8 @@ main(int argc, char **argv) return (errno); testfile = strdup(argv[1]); + if (testfile == NULL) + return (errno); fd = open(testfile, O_CREAT | O_RDWR, mode); if (fd < 0) { diff --git a/tests/zfs-tests/cmd/nvlist_to_lua.c b/tests/zfs-tests/cmd/nvlist_to_lua.c index b65b3fd26..3da69397a 100644 --- a/tests/zfs-tests/cmd/nvlist_to_lua.c +++ b/tests/zfs-tests/cmd/nvlist_to_lua.c @@ -129,6 +129,11 @@ run_tests(void) /* Note: maximum nvlist key length is 32KB */ int len = 1024 * 31; char *bigstring = malloc(len); + if (bigstring == NULL) { + perror("malloc"); + exit(EXIT_FAILURE); + } + for (int i = 0; i < len; i++) bigstring[i] = 'a' + i % 26; bigstring[len - 1] = '\0';