Author: asomers
Date: Tue Apr 12 19:11:14 2016
New Revision: 297868
URL: https://svnweb.freebsd.org/changeset/base/297868
Log:
Fix rare double free in vdev_geom_attrchanged
sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Don't drop the g_topology_lock before freeing old_physpath. That
opens up a race where one thread can call vdev_geom_attrchanged,
set old_physpath, drop the g_topology_lock, then block trying to
acquire the SCL_STATE lock. Then another thread can come into
vdev_geom_attrchanged, set old_physpath to the same value, and
proceed to free it. When the first thread resumes, it will free
the same location.
It turns out that the SCL_STATE lock isn't needed. It was
originally added by gibbs to protect vd->vdev_physpath while
updating the same. However, the update process subsequently was
switched to an atomic operation (a pointer swap). Now, there is
no need for the SCL_STATE lock, and hence no need to drop the
g_topology_lock.
Reviewed by: delphij
MFC after: 4 weeks
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D5413
Modified:
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==============================================================================
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Tue Apr
12 18:50:37 2016 (r297867)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Tue Apr
12 19:11:14 2016 (r297868)
@@ -115,27 +115,14 @@ vdev_geom_attrchanged(struct g_consumer
if (error == 0) {
char *old_physpath;
+ /* g_topology lock ensures that vdev has not been closed */
+ g_topology_assert();
old_physpath = vd->vdev_physpath;
vd->vdev_physpath = spa_strdup(physpath);
spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE);
- if (old_physpath != NULL) {
- int held_lock;
-
- held_lock = spa_config_held(spa, SCL_STATE, RW_WRITER);
- if (held_lock == 0) {
- g_topology_unlock();
- spa_config_enter(spa, SCL_STATE, FTAG,
- RW_WRITER);
- }
-
+ if (old_physpath != NULL)
spa_strfree(old_physpath);
-
- if (held_lock == 0) {
- spa_config_exit(spa, SCL_STATE, FTAG);
- g_topology_lock();
- }
- }
}
g_free(physpath);
}
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"
