From c562bbefc00f28334c2392626a27c34eab2f5b3e Mon Sep 17 00:00:00 2001 From: Richard Yao Date: Fri, 30 Sep 2022 19:59:51 -0400 Subject: [PATCH] Fix potential NULL pointer dereference in dsl_dataset_promote_check() If the `list_head()` returns NULL, we dereference it, right before we check to see if it returned NULL. We have defined two different pointers that both point to the same thing, which are `origin_head` and `origin_ds`. Almost everything uses `origin_ds`, so we switch them to use `origin_ds`. We also promote `origin_ds` to a const pointer so that the compiler verifies that nothing modifies it. Coverity complained about this. Reviewed-by: Brian Behlendorf Reviewed-by: Neal Gompa Signed-off-by: Richard Yao Closes #13967 --- module/zfs/dsl_dataset.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/module/zfs/dsl_dataset.c b/module/zfs/dsl_dataset.c index e8692c9543..0a460213de 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -3307,7 +3307,6 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) dsl_pool_t *dp = dmu_tx_pool(tx); dsl_dataset_t *hds; struct promotenode *snap; - dsl_dataset_t *origin_ds, *origin_head; int err; uint64_t unused; uint64_t ss_mv_cnt; @@ -3327,12 +3326,11 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) } snap = list_head(&ddpa->shared_snaps); - origin_head = snap->ds; if (snap == NULL) { err = SET_ERROR(ENOENT); goto out; } - origin_ds = snap->ds; + dsl_dataset_t *const origin_ds = snap->ds; /* * Encrypted clones share a DSL Crypto Key with their origin's dsl dir. @@ -3428,10 +3426,10 @@ dsl_dataset_promote_check(void *arg, dmu_tx_t *tx) * Check that bookmarks that are being transferred don't have * name conflicts. */ - for (dsl_bookmark_node_t *dbn = avl_first(&origin_head->ds_bookmarks); + for (dsl_bookmark_node_t *dbn = avl_first(&origin_ds->ds_bookmarks); dbn != NULL && dbn->dbn_phys.zbm_creation_txg <= dsl_dataset_phys(origin_ds)->ds_creation_txg; - dbn = AVL_NEXT(&origin_head->ds_bookmarks, dbn)) { + dbn = AVL_NEXT(&origin_ds->ds_bookmarks, dbn)) { if (strlen(dbn->dbn_name) >= max_snap_len) { err = SET_ERROR(ENAMETOOLONG); goto out;