Author: asomers
Date: Tue May 10 16:49:50 2016
New Revision: 299376
URL: https://svnweb.freebsd.org/changeset/base/299376
Log:
MFC 297868
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.
Modified:
stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Directory Properties:
stable/10/ (props changed)
Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==============================================================================
--- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Tue May 10 16:40:19 2016 (r299375)
+++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Tue May 10 16:49:50 2016 (r299376)
@@ -110,27 +110,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"
