c/mirror_zfs
1
0
Fork 0
mirror of https://git.proxmox.com/git/mirror_zfs.git synced 2024-12-27 11:29:36 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix zvol_state_t->zv_open_count race

Browse Source 
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 <bprotopopov@hotmail.com>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov
Signed-off-by: loli10K <ezomori.nozomu@gmail.com>
Closes #6213
This commit is contained in:
LOLi 2017-06-15 20:08:45 +02:00 committed by Brian Behlendorf
parent 627791f3c0
commit 97f8d7961e
1 changed files with 21 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
module/zfs/zvol.c
View File

@ -1964,22 +1964,27 @@ zvol_remove_minors_impl(const char *name)
(strncmp(zv->zv_name, name, namelen) == 0 && (strncmp(zv->zv_name, name, namelen) == 0 &&
(zv->zv_name[namelen] == '/' || (zv->zv_name[namelen] == '/' ||
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 * By taking zv_state_lock here, we guarantee that no
* one is currently using this zv * one is currently using this zv
*/ */
mutex_enter(&zv->zv_state_lock); mutex_enter(&zv->zv_state_lock);
zvol_remove(zv);
/* If in use, leave alone */
if (zv->zv_open_count > 0 ||
atomic_read(&zv->zv_suspend_ref)) {
mutex_exit(&zv->zv_state_lock); mutex_exit(&zv->zv_state_lock);
continue;
}
zvol_remove(zv);
/* clear this so zvol_open won't open it */ /* clear this so zvol_open won't open it */
zv->zv_disk->private_data = NULL; 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 */ /* try parallel zv_free, if failed do it in place */
t = taskq_dispatch(system_taskq, zvol_free, zv, t = taskq_dispatch(system_taskq, zvol_free, zv,
TQ_SLEEP); TQ_SLEEP);
@ -2021,19 +2026,24 @@ zvol_remove_minor_impl(const char *name)
zv_next = list_next(&zvol_state_list, zv); zv_next = list_next(&zvol_state_list, zv);
if (strcmp(zv->zv_name, name) == 0) { 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 * By taking zv_state_lock here, we guarantee that no
* one is currently using this zv * one is currently using this zv
*/ */
mutex_enter(&zv->zv_state_lock); mutex_enter(&zv->zv_state_lock);
zvol_remove(zv);
/* If in use, leave alone */
if (zv->zv_open_count > 0 ||
atomic_read(&zv->zv_suspend_ref)) {
mutex_exit(&zv->zv_state_lock); mutex_exit(&zv->zv_state_lock);
continue;
}
zvol_remove(zv);
/* clear this so zvol_open won't open it */ /* clear this so zvol_open won't open it */
zv->zv_disk->private_data = NULL; zv->zv_disk->private_data = NULL;
mutex_exit(&zv->zv_state_lock);
break; break;
} }
} }