From 3af336e901a1c382d23cee9bd48dcbf5b526d644 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Sat, 15 Oct 2022 22:54:57 -0400 Subject: [PATCH 1/6] Cleanup: Simplify userspace abd_free_chunks() Clang's static analyzer complained that we could use after free here if the inner loop ever iterated. That is a false positive, but upon inspection, the userland abd_alloc_chunks() function never will put multiple consecutive pages into a `struct scatterlist`, so there is no need to loop. We delete the inner loop. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- module/os/linux/zfs/abd_os.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/module/os/linux/zfs/abd_os.c b/module/os/linux/zfs/abd_os.c index 2ab85f8cccd0..66973a0a59bf 100644 --- a/module/os/linux/zfs/abd_os.c +++ b/module/os/linux/zfs/abd_os.c @@ -597,10 +597,8 @@ abd_free_chunks(abd_t *abd) struct scatterlist *sg; abd_for_each_sg(abd, sg, n, i) { - for (int j = 0; j < sg->length; j += PAGESIZE) { - struct page *p = nth_page(sg_page(sg), j >> PAGE_SHIFT); - umem_free(p, PAGESIZE); - } + struct page *p = nth_page(sg_page(sg), 0); + umem_free(p, PAGESIZE); } abd_free_sg_table(abd); } From bf7595fe75b31b74a1500111f1278b0234138714 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Sun, 16 Oct 2022 00:42:03 -0400 Subject: [PATCH 2/6] Cleanup: Delete unnecessary pointer check from vdev_to_nvlist_iter() This confused Clang's static analyzer, making it think there was a possible NULL pointer dereference. There is no NULL pointer dereference. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- lib/libzfs/libzfs_pool.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libzfs/libzfs_pool.c b/lib/libzfs/libzfs_pool.c index c6f31d785b89..f0bb2370a4c8 100644 --- a/lib/libzfs/libzfs_pool.c +++ b/lib/libzfs/libzfs_pool.c @@ -2678,7 +2678,7 @@ vdev_to_nvlist_iter(nvlist_t *nv, nvlist_t *search, boolean_t *avail_spare, if (zfs_strcmp_pathname(srchval, val, wholedisk) == 0) return (nv); - } else if (strcmp(srchkey, ZPOOL_CONFIG_TYPE) == 0 && val) { + } else if (strcmp(srchkey, ZPOOL_CONFIG_TYPE) == 0) { char *type, *idx, *end, *p; uint64_t id, vdev_id; From a02f1aac787b1feb6ad9008f02e5b49134c77ec3 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Mon, 17 Oct 2022 02:02:12 -0400 Subject: [PATCH 3/6] Cleanup: metaslab_alloc_dva() should not NULL check mg->mg_next This is a circularly linked list. mg->mg_next can never be NULL. This caused 3 defect reports in Coverity. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- module/zfs/metaslab.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module/zfs/metaslab.c b/module/zfs/metaslab.c index 3c32d6f051e8..c624833bc981 100644 --- a/module/zfs/metaslab.c +++ b/module/zfs/metaslab.c @@ -5131,8 +5131,7 @@ metaslab_alloc_dva(spa_t *spa, metaslab_class_t *mc, uint64_t psize, if (vd != NULL && vd->vdev_mg != NULL) { mg = vdev_get_mg(vd, mc); - if (flags & METASLAB_HINTBP_AVOID && - mg->mg_next != NULL) + if (flags & METASLAB_HINTBP_AVOID) mg = mg->mg_next; } else { mg = mca->mca_rotor; From dba4be4f751c85bcffd2a70e7f34c97f2fa955b3 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Mon, 17 Oct 2022 02:18:09 -0400 Subject: [PATCH 4/6] Cleanup: zvol_add_clones() should not NULL check dp It is never NULL because we return early if dsl_pool_hold() fails. This caused Coverity to complain. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- module/zfs/zvol.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index 2e2860ff0212..be8ee34f27ae 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -1026,8 +1026,7 @@ zvol_add_clones(const char *dsname, list_t *minors_list) out: if (dd != NULL) dsl_dir_rele(dd, FTAG); - if (dp != NULL) - dsl_pool_rele(dp, FTAG); + dsl_pool_rele(dp, FTAG); } /* From 963765b9ef2ecc69e2b105bf256cfed638668a58 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Mon, 17 Oct 2022 02:20:21 -0400 Subject: [PATCH 5/6] Cleanup: Delete dead code from send_merge_thread() range is always deferenced before it reaches this check, such that the kmem_zalloc() call is never executed. There is also no need to set `range->eos_marker = B_TRUE` because it is already set. Coverity incorrectly complained about a potential NULL pointer dereference because of this. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- module/zfs/dmu_send.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index 4ee3ffc352b8..ceef87a3002e 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -1586,9 +1586,8 @@ send_merge_thread(void *arg) } range_free(front_ranges[i]); } - if (range == NULL) - range = kmem_zalloc(sizeof (*range), KM_SLEEP); - range->eos_marker = B_TRUE; + ASSERT3P(range, !=, NULL); + ASSERT3S(range->eos_marker, ==, B_TRUE); bqueue_enqueue_flush(&smt_arg->q, range, 1); spl_fstrans_unmark(cookie); thread_exit(); From ad3f36da06c29af33b303af614278c3b65901c59 Mon Sep 17 00:00:00 2001 From: Richard Yao <richard.yao@alumni.stonybrook.edu> Date: Mon, 17 Oct 2022 17:57:59 -0400 Subject: [PATCH 6/6] Cleanup: Remove NULL pointer check from dmu_send_impl() The pointer is to a structure member, so it is never NULL. Coverity complained about this. Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu> --- module/zfs/dmu_send.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module/zfs/dmu_send.c b/module/zfs/dmu_send.c index ceef87a3002e..767b341a4347 100644 --- a/module/zfs/dmu_send.c +++ b/module/zfs/dmu_send.c @@ -2510,8 +2510,7 @@ dmu_send_impl(struct dmu_send_params *dspp) } if (featureflags & DMU_BACKUP_FEATURE_RAW) { - uint64_t ivset_guid = (ancestor_zb != NULL) ? - ancestor_zb->zbm_ivset_guid : 0; + uint64_t ivset_guid = ancestor_zb->zbm_ivset_guid; nvlist_t *keynvl = NULL; ASSERT(os->os_encrypted);