diff options
author | asomers <asomers@FreeBSD.org> | 2016-05-10 16:49:50 +0000 |
---|---|---|
committer | asomers <asomers@FreeBSD.org> | 2016-05-10 16:49:50 +0000 |
commit | 2dcc4d752ae214de606b7ef37b75e4cca6dff15f (patch) | |
tree | 5cca6f3158a09b58375d21b6a304047c359f17a0 /sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c | |
parent | bbbec9298fca2d2cc57250970838872a38d28386 (diff) | |
download | FreeBSD-src-2dcc4d752ae214de606b7ef37b75e4cca6dff15f.zip FreeBSD-src-2dcc4d752ae214de606b7ef37b75e4cca6dff15f.tar.gz |
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.
Diffstat (limited to 'sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c')
-rw-r--r-- | sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c | 19 |
1 files changed, 3 insertions, 16 deletions
diff --git a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c index 30eb903..acc364a 100644 --- a/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c +++ b/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c @@ -110,27 +110,14 @@ vdev_geom_attrchanged(struct g_consumer *cp, const char *attr) 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); } |