From a36b37d4de5d90bf3016a7ca23686c3295f6b01a 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 c944db2c5..7a066b786 100644 --- a/module/zfs/dsl_dataset.c +++ b/module/zfs/dsl_dataset.c @@ -3285,7 +3285,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; @@ -3305,12 +3304,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. @@ -3406,10 +3404,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;