From a6ccb36b948efed0eeee4fcf99fe4b5fb81ae1d5 Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Wed, 12 Oct 2022 14:25:18 -0400 Subject: [PATCH] Add defensive assertions Coverity complains about possible bugs involving referencing NULL return values and division by zero. The division by zero bugs require that a block pointer be corrupt, either from in-memory corruption, or on-disk corruption. The NULL return value complaints are only bugs if assumptions that we make about the state of data structures are wrong. Some seem impossible to be wrong and thus are false positives, while others are hard to analyze. Rather than dismiss these as false positives by assuming we know better, we add defensive assertions to let us know when our assumptions are wrong. Reviewed-by: Brian Behlendorf Signed-off-by: Richard Yao Closes #13972 --- cmd/zdb/zdb_il.c | 1 + module/os/freebsd/zfs/zfs_acl.c | 1 + module/os/linux/zfs/zfs_acl.c | 1 + module/zfs/arc.c | 1 + module/zfs/dbuf.c | 2 ++ module/zfs/dmu_traverse.c | 1 + module/zfs/dsl_deadlist.c | 1 + module/zfs/dsl_scan.c | 1 + module/zfs/mmp.c | 4 +++- module/zfs/range_tree.c | 1 + module/zfs/spa_log_spacemap.c | 1 + module/zfs/vdev_indirect.c | 1 + module/zfs/vdev_queue.c | 1 + module/zfs/zil.c | 1 + 14 files changed, 17 insertions(+), 1 deletion(-) diff --git a/cmd/zdb/zdb_il.c b/cmd/zdb/zdb_il.c index 4de7619e4bde..02cc10fb7817 100644 --- a/cmd/zdb/zdb_il.c +++ b/cmd/zdb/zdb_il.c @@ -182,6 +182,7 @@ zil_prt_rec_write(zilog_t *zilog, int txtype, const void *arg) return; } + ASSERT3U(BP_GET_LSIZE(bp), !=, 0); SET_BOOKMARK(&zb, dmu_objset_id(zilog->zl_os), lr->lr_foid, ZB_ZIL_LEVEL, lr->lr_offset / BP_GET_LSIZE(bp)); diff --git a/module/os/freebsd/zfs/zfs_acl.c b/module/os/freebsd/zfs/zfs_acl.c index 963102f3b62a..a85d4c24178b 100644 --- a/module/os/freebsd/zfs/zfs_acl.c +++ b/module/os/freebsd/zfs/zfs_acl.c @@ -1133,6 +1133,7 @@ zfs_acl_data_locator(void **dataptr, uint32_t *length, uint32_t buflen, cb->cb_acl_node = list_next(&cb->cb_aclp->z_acl, cb->cb_acl_node); } + ASSERT3P(cb->cb_acl_node, !=, NULL); *dataptr = cb->cb_acl_node->z_acldata; *length = cb->cb_acl_node->z_size; } diff --git a/module/os/linux/zfs/zfs_acl.c b/module/os/linux/zfs/zfs_acl.c index 5935403b49d0..d5cd5de890ab 100644 --- a/module/os/linux/zfs/zfs_acl.c +++ b/module/os/linux/zfs/zfs_acl.c @@ -1163,6 +1163,7 @@ zfs_acl_data_locator(void **dataptr, uint32_t *length, uint32_t buflen, cb->cb_acl_node = list_next(&cb->cb_aclp->z_acl, cb->cb_acl_node); } + ASSERT3P(cb->cb_acl_node, !=, NULL); *dataptr = cb->cb_acl_node->z_acldata; *length = cb->cb_acl_node->z_size; } diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 33865f715b0f..9254d8b84902 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -8539,6 +8539,7 @@ l2arc_dev_get_next(void) else if (next == first) break; + ASSERT3P(next, !=, NULL); } while (vdev_is_dead(next->l2ad_vdev) || next->l2ad_rebuild || next->l2ad_trim_all); diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index db1123d37d98..85de95baf61a 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -2687,6 +2687,7 @@ dbuf_override_impl(dmu_buf_impl_t *db, const blkptr_t *bp, dmu_tx_t *tx) dbuf_dirty_record_t *dr; dr = list_head(&db->db_dirty_records); + ASSERT3P(dr, !=, NULL); ASSERT3U(dr->dr_txg, ==, tx->tx_txg); dl = &dr->dt.dl; dl->dr_overridden_by = *bp; @@ -2748,6 +2749,7 @@ dmu_buf_write_embedded(dmu_buf_t *dbuf, void *data, dmu_buf_will_not_fill(dbuf, tx); dr = list_head(&db->db_dirty_records); + ASSERT3P(dr, !=, NULL); ASSERT3U(dr->dr_txg, ==, tx->tx_txg); dl = &dr->dt.dl; encode_embedded_bp_compressed(&dl->dr_overridden_by, diff --git a/module/zfs/dmu_traverse.c b/module/zfs/dmu_traverse.c index 2ed75640f68d..d712133cd789 100644 --- a/module/zfs/dmu_traverse.c +++ b/module/zfs/dmu_traverse.c @@ -111,6 +111,7 @@ traverse_zil_record(zilog_t *zilog, const lr_t *lrc, void *arg, if (claim_txg == 0 || bp->blk_birth < claim_txg) return (0); + ASSERT3U(BP_GET_LSIZE(bp), !=, 0); SET_BOOKMARK(&zb, td->td_objset, lr->lr_foid, ZB_ZIL_LEVEL, lr->lr_offset / BP_GET_LSIZE(bp)); diff --git a/module/zfs/dsl_deadlist.c b/module/zfs/dsl_deadlist.c index 1ecae0fe3865..031be59e31cc 100644 --- a/module/zfs/dsl_deadlist.c +++ b/module/zfs/dsl_deadlist.c @@ -542,6 +542,7 @@ dsl_deadlist_remove_key(dsl_deadlist_t *dl, uint64_t mintxg, dmu_tx_t *tx) dle = avl_find(&dl->dl_tree, &dle_tofind, NULL); ASSERT3P(dle, !=, NULL); dle_prev = AVL_PREV(&dl->dl_tree, dle); + ASSERT3P(dle_prev, !=, NULL); dle_enqueue_subobj(dl, dle_prev, dle->dle_bpobj.bpo_object, tx); diff --git a/module/zfs/dsl_scan.c b/module/zfs/dsl_scan.c index f0cd1feaf55b..09e271395cc4 100644 --- a/module/zfs/dsl_scan.c +++ b/module/zfs/dsl_scan.c @@ -1470,6 +1470,7 @@ dsl_scan_zil_record(zilog_t *zilog, const lr_t *lrc, void *arg, if (claim_txg == 0 || bp->blk_birth < claim_txg) return (0); + ASSERT3U(BP_GET_LSIZE(bp), !=, 0); SET_BOOKMARK(&zb, zh->zh_log.blk_cksum.zc_word[ZIL_ZC_OBJSET], lr->lr_foid, ZB_ZIL_LEVEL, lr->lr_offset / BP_GET_LSIZE(bp)); diff --git a/module/zfs/mmp.c b/module/zfs/mmp.c index 92fd6c422330..b6d71aa8a458 100644 --- a/module/zfs/mmp.c +++ b/module/zfs/mmp.c @@ -303,8 +303,10 @@ mmp_next_leaf(spa_t *spa) do { leaf = list_next(&spa->spa_leaf_list, leaf); - if (leaf == NULL) + if (leaf == NULL) { leaf = list_head(&spa->spa_leaf_list); + ASSERT3P(leaf, !=, NULL); + } /* * We skip unwritable, offline, detached, and dRAID spare diff --git a/module/zfs/range_tree.c b/module/zfs/range_tree.c index a2923d1664c7..894c30fcae16 100644 --- a/module/zfs/range_tree.c +++ b/module/zfs/range_tree.c @@ -369,6 +369,7 @@ range_tree_add_impl(void *arg, uint64_t start, uint64_t size, uint64_t fill) * invalid as soon as we do any mutating btree operations. */ rs_after = zfs_btree_find(&rt->rt_root, &tmp, &where_after); + ASSERT3P(rs_after, !=, NULL); rs_set_start_raw(rs_after, rt, before_start); rs_set_fill(rs_after, rt, after_fill + before_fill + fill); rs = rs_after; diff --git a/module/zfs/spa_log_spacemap.c b/module/zfs/spa_log_spacemap.c index 4ecce8214f6a..5ce60bf212f1 100644 --- a/module/zfs/spa_log_spacemap.c +++ b/module/zfs/spa_log_spacemap.c @@ -507,6 +507,7 @@ void spa_log_summary_decrement_blkcount(spa_t *spa, uint64_t blocks_gone) { log_summary_entry_t *e = list_head(&spa->spa_log_summary); + ASSERT3P(e, !=, NULL); if (e->lse_txgcount > 0) e->lse_txgcount--; for (; e != NULL; e = list_head(&spa->spa_log_summary)) { diff --git a/module/zfs/vdev_indirect.c b/module/zfs/vdev_indirect.c index 0ca0c245e952..b0bd71c58e69 100644 --- a/module/zfs/vdev_indirect.c +++ b/module/zfs/vdev_indirect.c @@ -1319,6 +1319,7 @@ vdev_indirect_io_start(zio_t *zio) vdev_indirect_gather_splits, zio); indirect_split_t *first = list_head(&iv->iv_splits); + ASSERT3P(first, !=, NULL); if (first->is_size == zio->io_size) { /* * This is not a split block; we are pointing to the entire diff --git a/module/zfs/vdev_queue.c b/module/zfs/vdev_queue.c index 1acb89cea393..f4e8d1427d7c 100644 --- a/module/zfs/vdev_queue.c +++ b/module/zfs/vdev_queue.c @@ -756,6 +756,7 @@ vdev_queue_aggregate(vdev_queue_t *vq, zio_t *zio) do { dio = nio; nio = AVL_NEXT(t, dio); + ASSERT3P(dio, !=, NULL); zio_add_child(dio, aio); vdev_queue_io_remove(vq, dio); diff --git a/module/zfs/zil.c b/module/zfs/zil.c index dc5b8018e16e..075bac832f06 100644 --- a/module/zfs/zil.c +++ b/module/zfs/zil.c @@ -339,6 +339,7 @@ zil_read_log_data(zilog_t *zilog, const lr_write_t *lr, void *wbuf) if (wbuf == NULL) zio_flags |= ZIO_FLAG_RAW; + ASSERT3U(BP_GET_LSIZE(bp), !=, 0); SET_BOOKMARK(&zb, dmu_objset_id(zilog->zl_os), lr->lr_foid, ZB_ZIL_LEVEL, lr->lr_offset / BP_GET_LSIZE(bp));