Skip to content

Commit

Permalink
Fix potential NULL pointer dereference in dsl_dataset_promote_check()
Browse files Browse the repository at this point in the history
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 <[email protected]>
Reviewed-by: Neal Gompa <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes openzfs#13967
  • Loading branch information
ryao authored and tonyhutter committed Nov 22, 2022
1 parent a11799d commit 71533e4
Showing 1 changed file with 3 additions and 5 deletions.
8 changes: 3 additions & 5 deletions module/zfs/dsl_dataset.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 71533e4

Please sign in to comment.