diff options
| author | Richard Yao <[email protected]> | 2022-09-27 19:45:51 -0400 |
|---|---|---|
| committer | GitHub <[email protected]> | 2022-09-27 16:45:51 -0700 |
| commit | 88b199c24e789f3680193d2f41101f75efd8803f (patch) | |
| tree | a820c014c5d0bceeb809a433f5bedde4a93364eb /module | |
| parent | 31b4e008f13fe00c5619fee06c6502d417448bd5 (diff) | |
Cleanup spa_export_common()
Coverity complains about a possible NULL pointer dereference. This is
impossible, but it suspects it because we do a NULL check against
`spa->spa_root_vdev`. This NULL check was never necessary and makes the
code harder to understand, so we drop it.
In particular, we dereference `spa->spa_root_vdev` when `new_state !=
POOL_STATE_UNINITIALIZED && !hardforce`. The first is only true when
spa_reset is called, which only occurs under fault injection. The
second is true unless `zpool export -F $POOLNAME` is used. Therefore,
we effectively *always* dereference the pointer. In the cases where we
do not, there is no reason to think it is unsafe. Therefore this change
is safe to make.
Reviewed-by: Brian Behlendorf <[email protected]>
Signed-off-by: Richard Yao <[email protected]>
Closes #13905
Diffstat (limited to 'module')
| -rw-r--r-- | module/zfs/spa.c | 14 |
1 files changed, 6 insertions, 8 deletions
diff --git a/module/zfs/spa.c b/module/zfs/spa.c index 10efb36f0..b8e054a5f 100644 --- a/module/zfs/spa.c +++ b/module/zfs/spa.c @@ -6427,6 +6427,7 @@ spa_export_common(const char *pool, int new_state, nvlist_t **oldconfig, } if (spa->spa_sync_on) { + vdev_t *rvd = spa->spa_root_vdev; /* * A pool cannot be exported if it has an active shared spare. * This is to prevent other pools stealing the active spare @@ -6446,13 +6447,10 @@ spa_export_common(const char *pool, int new_state, nvlist_t **oldconfig, * dirty data resulting from the initialization is * committed to disk before we unload the pool. */ - if (spa->spa_root_vdev != NULL) { - vdev_t *rvd = spa->spa_root_vdev; - vdev_initialize_stop_all(rvd, VDEV_INITIALIZE_ACTIVE); - vdev_trim_stop_all(rvd, VDEV_TRIM_ACTIVE); - vdev_autotrim_stop_all(spa); - vdev_rebuild_stop_all(spa); - } + vdev_initialize_stop_all(rvd, VDEV_INITIALIZE_ACTIVE); + vdev_trim_stop_all(rvd, VDEV_TRIM_ACTIVE); + vdev_autotrim_stop_all(spa); + vdev_rebuild_stop_all(spa); /* * We want this to be reflected on every label, @@ -6462,7 +6460,7 @@ spa_export_common(const char *pool, int new_state, nvlist_t **oldconfig, if (new_state != POOL_STATE_UNINITIALIZED && !hardforce) { spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER); spa->spa_state = new_state; - vdev_config_dirty(spa->spa_root_vdev); + vdev_config_dirty(rvd); spa_config_exit(spa, SCL_ALL, FTAG); } |