From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
To: Tyler Hicks <tyhicks@linux.microsoft.com>,
Pavel Tatashin <pasha.tatashin@soleen.com>,
Petr Vorel <pvorel@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
Stefan Haberland <sth@linux.ibm.com>,
Jan Hoeppner <hoeppner@linux.ibm.com>,
linux-block <linux-block@vger.kernel.org>,
syzbot <syzbot+61e04e51b7ac86930589@syzkaller.appspotmail.com>,
Tejun Heo <tj@kernel.org>, Jan Kara <jack@suse.cz>
Subject: [PATCH] loop: drop loop_ctl_mutex around del_gendisk() in loop_remove()
Date: Sat, 12 Jun 2021 00:14:20 +0900 [thread overview]
Message-ID: <d15e9392-44d0-f42c-cbac-859459a99395@i-love.sakura.ne.jp> (raw)
In-Reply-To: <4e315cc1-cbb9-b00e-c4cd-ca4f6f60f202@i-love.sakura.ne.jp>
syzbot is reporting circular locking dependency between loop_ctl_mutex and
bdev->bd_mutex [1] due to commit c76f48eb5c084b1e ("block: take bd_mutex
around delete_partitions in del_gendisk").
But calling del_gendisk() from loop_remove() without loop_ctl_mutex held
triggers a different race problem regarding sysfs entry management. We
somehow need to serialize "add_disk() from loop_add()" and "del_gendisk()
from loop_remove()". Fortunately, since loop_control_ioctl() is called
with no locks held, we can use "sleep and retry" approach without risking
deadlock.
Since "struct loop_device"->lo_disk->private_data is set to non-NULL at
loop_add() and is reset to NULL before calling loop_remove(), we can use
it as a flag for taking appropriate action ("sleep and retry" or "skip")
when loop_remove() is in progress.
Link: https://syzkaller.appspot.com/bug?extid=61e04e51b7ac86930589 [1]
Reported-by: syzbot <syzbot+61e04e51b7ac86930589@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot <syzbot+61e04e51b7ac86930589@syzkaller.appspotmail.com>
Fixes: c76f48eb5c084b1e ("block: take bd_mutex around delete_partitions in del_gendisk")
---
drivers/block/loop.c | 18 ++++++++++++++++--
1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index d58d68f3c7cd..d4c9567b2904 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -2188,7 +2188,9 @@ static int loop_add(struct loop_device **l, int i)
static void loop_remove(struct loop_device *lo)
{
+ mutex_unlock(&loop_ctl_mutex);
del_gendisk(lo->lo_disk);
+ mutex_lock(&loop_ctl_mutex);
blk_cleanup_queue(lo->lo_queue);
blk_mq_free_tag_set(&lo->tag_set);
put_disk(lo->lo_disk);
@@ -2201,7 +2203,7 @@ static int find_free_cb(int id, void *ptr, void *data)
struct loop_device *lo = ptr;
struct loop_device **l = data;
- if (lo->lo_state == Lo_unbound) {
+ if (lo->lo_state == Lo_unbound && lo->lo_disk->private_data) {
*l = lo;
return 1;
}
@@ -2254,6 +2256,13 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
struct loop_device *lo;
int ret;
+ if (0) {
+unlock_and_retry:
+ mutex_unlock(&loop_ctl_mutex);
+ if (schedule_timeout_killable(HZ / 10))
+ return -EINTR;
+ }
+ debug_check_no_locks_held();
ret = mutex_lock_killable(&loop_ctl_mutex);
if (ret)
return ret;
@@ -2263,6 +2272,8 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
case LOOP_CTL_ADD:
ret = loop_lookup(&lo, parm);
if (ret >= 0) {
+ if (!lo->lo_disk->private_data)
+ goto unlock_and_retry;
ret = -EEXIST;
break;
}
@@ -2272,6 +2283,8 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
ret = loop_lookup(&lo, parm);
if (ret < 0)
break;
+ if (!lo->lo_disk->private_data)
+ goto unlock_and_retry;
ret = mutex_lock_killable(&lo->lo_mutex);
if (ret)
break;
@@ -2286,9 +2299,10 @@ static long loop_control_ioctl(struct file *file, unsigned int cmd,
break;
}
lo->lo_disk->private_data = NULL;
+ parm = lo->lo_number;
mutex_unlock(&lo->lo_mutex);
- idr_remove(&loop_index_idr, lo->lo_number);
loop_remove(lo);
+ idr_remove(&loop_index_idr, parm);
break;
case LOOP_CTL_GET_FREE:
ret = loop_lookup(&lo, -1);
--
2.25.1
next prev parent reply other threads:[~2021-06-11 15:14 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-13 17:33 [syzbot] possible deadlock in del_gendisk syzbot
2021-04-13 17:41 ` Steven Rostedt
2021-04-13 17:43 ` Steven Rostedt
2021-04-13 18:24 ` Dmitry Vyukov
2021-04-13 18:40 ` Steven Rostedt
2021-04-13 18:43 ` Steven Rostedt
2021-04-16 7:51 ` Bisections with different bug manifestations Dmitry Vyukov
2021-04-16 13:13 ` Steven Rostedt
2021-04-16 13:26 ` Dmitry Vyukov
2021-04-16 13:48 ` Dmitry Vyukov
2021-06-07 10:56 ` [syzbot] possible deadlock in del_gendisk Tetsuo Handa
2021-06-09 16:31 ` Tetsuo Handa
2021-06-09 16:46 ` Tyler Hicks
2021-06-10 13:37 ` Tetsuo Handa
2021-06-11 6:46 ` Tetsuo Handa
2021-06-11 15:14 ` Tetsuo Handa [this message]
2021-06-15 5:30 ` [PATCH] loop: drop loop_ctl_mutex around del_gendisk() in loop_remove() Tetsuo Handa
2021-06-15 5:31 ` syzbot
2021-06-15 9:04 ` Jan Kara
2021-06-11 14:47 ` [syzbot] possible deadlock in del_gendisk Tetsuo Handa
2021-06-11 15:11 ` Pavel Tatashin
2021-06-11 15:18 ` Pavel Tatashin
2021-06-11 15:49 ` Tetsuo Handa
2021-06-12 2:35 ` Tetsuo Handa
2021-06-13 11:01 ` Tetsuo Handa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d15e9392-44d0-f42c-cbac-859459a99395@i-love.sakura.ne.jp \
--to=penguin-kernel@i-love.sakura.ne.jp \
--cc=axboe@kernel.dk \
--cc=hch@lst.de \
--cc=hoeppner@linux.ibm.com \
--cc=jack@suse.cz \
--cc=linux-block@vger.kernel.org \
--cc=pasha.tatashin@soleen.com \
--cc=pvorel@suse.cz \
--cc=sth@linux.ibm.com \
--cc=syzbot+61e04e51b7ac86930589@syzkaller.appspotmail.com \
--cc=tj@kernel.org \
--cc=tyhicks@linux.microsoft.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).