Archive-Team
/
zfs
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 103 Wiki Activity

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 <behlendorf1@llnl.gov>
Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
Closes #13905
This commit is contained in:
Richard Yao 2022-09-27 19:45:51 -04:00 committed by GitHub
parent 31b4e008f1
commit 88b199c24e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 6 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
module/zfs/spa.c
View File

@ -6427,6 +6427,7 @@ spa_export_common(const char *pool, int new_state, nvlist_t **oldconfig,
} }
if (spa->spa_sync_on) { if (spa->spa_sync_on) {
vdev_t *rvd = spa->spa_root_vdev;
/* /*
* A pool cannot be exported if it has an active shared spare. * A pool cannot be exported if it has an active shared spare.
* This is to prevent other pools stealing the active 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 * dirty data resulting from the initialization is
* committed to disk before we unload the pool. * committed to disk before we unload the pool.
*/ */
if (spa->spa_root_vdev != NULL) { vdev_initialize_stop_all(rvd, VDEV_INITIALIZE_ACTIVE);
vdev_t *rvd = spa->spa_root_vdev; vdev_trim_stop_all(rvd, VDEV_TRIM_ACTIVE);
vdev_initialize_stop_all(rvd, VDEV_INITIALIZE_ACTIVE); vdev_autotrim_stop_all(spa);
vdev_trim_stop_all(rvd, VDEV_TRIM_ACTIVE); vdev_rebuild_stop_all(spa);
vdev_autotrim_stop_all(spa);
vdev_rebuild_stop_all(spa);
}
/* /*
* We want this to be reflected on every label, * 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) { if (new_state != POOL_STATE_UNINITIALIZED && !hardforce) {
spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER); spa_config_enter(spa, SCL_ALL, FTAG, RW_WRITER);
spa->spa_state = new_state; spa->spa_state = new_state;
vdev_config_dirty(spa->spa_root_vdev); vdev_config_dirty(rvd);
spa_config_exit(spa, SCL_ALL, FTAG); spa_config_exit(spa, SCL_ALL, FTAG);
} }