From 67395be0c2bd337f3b480d295a485117ac6bc61b Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Fri, 30 Sep 2022 20:02:57 -0400 Subject: [PATCH] Fix userland dereference NULL return value bugs * `zstream_do_token()` does not handle failures from `libzfs_init()` * `ztest_global_vars_to_zdb_args()` does not handle failures from `calloc()`. * `zfs_snapshot_nvl()` will pass an offset to a NULL pointer as a source to `strlcpy()` if the provided nvlist is `NULL`. We handle these by doing what the existing error handling does for other errors involving these functions. Coverity complained about these. It had complained about several more, but one was fixed by 570ca4441e0583c8dcb5c7179f5eb331d1172784 and another was a false positive. The remaining complaints labelled "dereferece null return vaue" involve fetching things stored in in-kernel data structures via `list_head()/list_next()`, `AVL_PREV()/AVL_NEXT()` and `zfs_btree_find()`. Most of them occur in void functions that have no error handling. They are much harder to analyze than the two fixed in this patch, so they are left for a follow-up patch. Reviewed-by: Brian Behlendorf Reviewed-by: Ryan Moeller Signed-off-by: Richard Yao Closes #13971 --- cmd/zstream/zstream_token.c | 6 +++++- cmd/ztest.c | 6 ++++++ lib/libzfs/libzfs_dataset.c | 2 ++ 3 files changed, 13 insertions(+), 1 deletion(-) diff --git a/cmd/zstream/zstream_token.c b/cmd/zstream/zstream_token.c index 23cc56dce3..795a372633 100644 --- a/cmd/zstream/zstream_token.c +++ b/cmd/zstream/zstream_token.c @@ -49,6 +49,7 @@ int zstream_do_token(int argc, char *argv[]) { char *resume_token = NULL; + libzfs_handle_t *hdl; if (argc < 2) { (void) fprintf(stderr, "Need to pass the resume token\n"); @@ -57,7 +58,10 @@ zstream_do_token(int argc, char *argv[]) resume_token = argv[1]; - libzfs_handle_t *hdl = libzfs_init(); + if ((hdl = libzfs_init()) == NULL) { + (void) fprintf(stderr, "%s\n", libzfs_error_init(errno)); + return (1); + } nvlist_t *resume_nvl = zfs_send_resume_token_to_nvlist(hdl, resume_token); diff --git a/cmd/ztest.c b/cmd/ztest.c index 033bcfd335..e630d33531 100644 --- a/cmd/ztest.c +++ b/cmd/ztest.c @@ -6637,6 +6637,8 @@ ztest_global_vars_to_zdb_args(void) { char **args = calloc(2*ztest_opts.zo_gvars_count + 1, sizeof (char *)); char **cur = args; + if (args == NULL) + return (NULL); for (size_t i = 0; i < ztest_opts.zo_gvars_count; i++) { *cur++ = (char *)"-o"; *cur++ = ztest_opts.zo_gvars[i]; @@ -6906,6 +6908,10 @@ ztest_run_zdb(const char *pool) ztest_get_zdb_bin(bin, len); char **set_gvars_args = ztest_global_vars_to_zdb_args(); + if (set_gvars_args == NULL) { + fatal(B_FALSE, "Failed to allocate memory in " + "ztest_global_vars_to_zdb_args(). Cannot run zdb.\n"); + } char *set_gvars_args_joined = join_strings(set_gvars_args, " "); free(set_gvars_args); diff --git a/lib/libzfs/libzfs_dataset.c b/lib/libzfs/libzfs_dataset.c index 3288268fb2..133b3b3588 100644 --- a/lib/libzfs/libzfs_dataset.c +++ b/lib/libzfs/libzfs_dataset.c @@ -4164,6 +4164,8 @@ zfs_snapshot_nvl(libzfs_handle_t *hdl, nvlist_t *snaps, nvlist_t *props) * same pool, as does lzc_snapshot (below). */ elem = nvlist_next_nvpair(snaps, NULL); + if (elem == NULL) + return (-1); (void) strlcpy(pool, nvpair_name(elem), sizeof (pool)); pool[strcspn(pool, "/@")] = '\0'; zpool_hdl = zpool_open(hdl, pool);