From 97f8d7961e0b6e282f5296c20d5af9746c9db688 Mon Sep 17 00:00:00 2001 From: LOLi Date: Thu, 15 Jun 2017 20:08:45 +0200 Subject: [PATCH] Fix zvol_state_t->zv_open_count race 5559ba0 added zv_state_lock to protect zvol_state_t internal data: this, however, doesn't guard zv->zv_open_count and zv->zv_disk->private_data in zvol_remove_minors_impl(). Fix this by taking zv->zv_state_lock before we check its zv_open_count. P1 (z_zvol) P2 (systemd-udevd) --- --- zvol_remove_minors_impl() : zv->zv_open_count==0 zvol_open() ->mutex_enter(zv_state_lock) : zv->zv_open_count++ ->mutex_exit(zv_state_lock) ->mutex_enter(zv->zv_state_lock) ->zvol_remove(zv) ->mutex_exit(zv->zv_state_lock) : zv->zv_disk->private_data = NULL ->zvol_free() -->ASSERT(zv->zv_open_count==0) * zvol_release() : zv = disk->private_data ->ASSERT(zv && zv->zv_open_count>0) * --- --- * ASSERT() fails Reviewed by: Boris Protopopov Reviewed-by: Brian Behlendorf Closes #6213 --- module/zfs/zvol.c | 32 +++++++++++++++++++++----------- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/module/zfs/zvol.c b/module/zfs/zvol.c index b2392305b..7730c28c6 100644 --- a/module/zfs/zvol.c +++ b/module/zfs/zvol.c @@ -1964,22 +1964,27 @@ zvol_remove_minors_impl(const char *name) (strncmp(zv->zv_name, name, namelen) == 0 && (zv->zv_name[namelen] == '/' || zv->zv_name[namelen] == '@'))) { - - /* If in use, leave alone */ - if (zv->zv_open_count > 0 || - atomic_read(&zv->zv_suspend_ref)) - continue; /* * By taking zv_state_lock here, we guarantee that no * one is currently using this zv */ mutex_enter(&zv->zv_state_lock); + + /* If in use, leave alone */ + if (zv->zv_open_count > 0 || + atomic_read(&zv->zv_suspend_ref)) { + mutex_exit(&zv->zv_state_lock); + continue; + } + zvol_remove(zv); - mutex_exit(&zv->zv_state_lock); /* clear this so zvol_open won't open it */ zv->zv_disk->private_data = NULL; + /* Drop zv_state_lock before zvol_free() */ + mutex_exit(&zv->zv_state_lock); + /* try parallel zv_free, if failed do it in place */ t = taskq_dispatch(system_taskq, zvol_free, zv, TQ_SLEEP); @@ -2021,19 +2026,24 @@ zvol_remove_minor_impl(const char *name) zv_next = list_next(&zvol_state_list, zv); if (strcmp(zv->zv_name, name) == 0) { - /* If in use, leave alone */ - if (zv->zv_open_count > 0 || - atomic_read(&zv->zv_suspend_ref)) - continue; /* * By taking zv_state_lock here, we guarantee that no * one is currently using this zv */ mutex_enter(&zv->zv_state_lock); + + /* If in use, leave alone */ + if (zv->zv_open_count > 0 || + atomic_read(&zv->zv_suspend_ref)) { + mutex_exit(&zv->zv_state_lock); + continue; + } zvol_remove(zv); - mutex_exit(&zv->zv_state_lock); + /* clear this so zvol_open won't open it */ zv->zv_disk->private_data = NULL; + + mutex_exit(&zv->zv_state_lock); break; } } -- 2.40.0