From: Jan Kara <jack@suse.cz>
To: Jens Axboe <axboe@kernel.dk>
Cc: <linux-block@vger.kernel.org>,
Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>,
Jan Kara <jack@suse.cz>
Subject: [PATCH 13/16] loop: Move loop_reread_partitions() out of loop_ctl_mutex
Date: Thu, 8 Nov 2018 14:01:13 +0100 [thread overview]
Message-ID: <20181108130116.12140-14-jack@suse.cz> (raw)
In-Reply-To: <20181108130116.12140-1-jack@suse.cz>
Calling loop_reread_partitions() under loop_ctl_mutex causes lockdep to
complain about circular lock dependency between bdev->bd_mutex and
lo->lo_ctl_mutex. The problem is that on loop device open or close
lo_open() and lo_release() get called with bdev->bd_mutex held and they
need to acquire loop_ctl_mutex. OTOH when loop_reread_partitions() is
called with loop_ctl_mutex held, it will call blkdev_reread_part() which
acquires bdev->bd_mutex. See syzbot report for details [1].
Move all calls of loop_rescan_partitions() out of loop_ctl_mutex to
avoid lockdep warning and fix deadlock possibility.
[1] https://syzkaller.appspot.com/bug?id=bf154052f0eea4bc7712499e4569505907d1588
Reported-by: syzbot <syzbot+4684a000d5abdade83fac55b1e7d1f935ef1936e@syzkaller.appspotmail.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
drivers/block/loop.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index f1d7a4fe30fc..cce5d4e8e863 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -679,6 +679,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
{
struct file *file, *old_file;
int error;
+ bool partscan;
error = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
if (error)
@@ -720,9 +721,10 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
blk_mq_unfreeze_queue(lo->lo_queue);
fput(old_file);
- if (lo->lo_flags & LO_FLAGS_PARTSCAN)
- loop_reread_partitions(lo, bdev);
+ partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
mutex_unlock(&loop_ctl_mutex);
+ if (partscan)
+ loop_reread_partitions(lo, bdev);
return 0;
out_putf:
@@ -903,6 +905,7 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
int lo_flags = 0;
int error;
loff_t size;
+ bool partscan;
/* This is safe, since we have a reference from open(). */
__module_get(THIS_MODULE);
@@ -969,14 +972,15 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
lo->lo_state = Lo_bound;
if (part_shift)
lo->lo_flags |= LO_FLAGS_PARTSCAN;
- if (lo->lo_flags & LO_FLAGS_PARTSCAN)
- loop_reread_partitions(lo, bdev);
+ partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
/* Grab the block_device to prevent its destruction after we
* put /dev/loopXX inode. Later in __loop_clr_fd() we bdput(bdev).
*/
bdgrab(bdev);
mutex_unlock(&loop_ctl_mutex);
+ if (partscan)
+ loop_reread_partitions(lo, bdev);
return 0;
out_unlock:
@@ -1157,6 +1161,8 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
int err;
struct loop_func_table *xfer;
kuid_t uid = current_uid();
+ struct block_device *bdev;
+ bool partscan = false;
err = mutex_lock_killable_nested(&loop_ctl_mutex, 1);
if (err)
@@ -1245,10 +1251,13 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
!(lo->lo_flags & LO_FLAGS_PARTSCAN)) {
lo->lo_flags |= LO_FLAGS_PARTSCAN;
lo->lo_disk->flags &= ~GENHD_FL_NO_PART_SCAN;
- loop_reread_partitions(lo, lo->lo_device);
+ bdev = lo->lo_device;
+ partscan = true;
}
out_unlock:
mutex_unlock(&loop_ctl_mutex);
+ if (partscan)
+ loop_reread_partitions(lo, bdev);
return err;
}
--
2.16.4
next prev parent reply other threads:[~2018-11-08 22:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-08 13:01 [PATCH 0/16 v3] loop: Fix oops and possible deadlocks Jan Kara
2018-11-08 13:01 ` [PATCH 01/16] block/loop: Don't grab "struct file" for vfs_getattr() operation Jan Kara
2018-11-08 13:01 ` [PATCH 02/16] block/loop: Use global lock for ioctl() operation Jan Kara
2018-11-08 13:01 ` [PATCH 03/16] loop: Fold __loop_release into loop_release Jan Kara
2018-11-08 13:01 ` [PATCH 04/16] loop: Get rid of loop_index_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 05/16] loop: Push lo_ctl_mutex down into individual ioctls Jan Kara
2018-11-08 13:01 ` [PATCH 06/16] loop: Split setting of lo_state from loop_clr_fd Jan Kara
2018-11-08 13:01 ` [PATCH 07/16] loop: Push loop_ctl_mutex down into loop_clr_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 08/16] loop: Push loop_ctl_mutex down to loop_get_status() Jan Kara
2018-11-08 13:01 ` [PATCH 09/16] loop: Push loop_ctl_mutex down to loop_set_status() Jan Kara
2018-11-08 13:01 ` [PATCH 10/16] loop: Push loop_ctl_mutex down to loop_set_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 11/16] loop: Push loop_ctl_mutex down to loop_change_fd() Jan Kara
2018-11-08 13:01 ` [PATCH 12/16] loop: Move special partition reread handling in loop_clr_fd() Jan Kara
2018-11-08 13:01 ` Jan Kara [this message]
2018-11-08 13:01 ` [PATCH 14/16] loop: Fix deadlock when calling blkdev_reread_part() Jan Kara
2018-11-08 13:01 ` [PATCH 15/16] loop: Avoid circular locking dependency between loop_ctl_mutex and bd_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 16/16] loop: Get rid of 'nested' acquisition of loop_ctl_mutex Jan Kara
2018-11-08 13:21 ` [PATCH 0/16 v3] loop: Fix oops and possible deadlocks Jens Axboe
2018-11-08 13:25 ` Jan Kara
2018-11-08 13:31 ` Jens Axboe
2018-11-08 21:28 ` Theodore Y. Ts'o
2018-11-12 10:15 ` Jan Kara
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=20181108130116.12140-14-jack@suse.cz \
--to=jack@suse.cz \
--cc=axboe@kernel.dk \
--cc=linux-block@vger.kernel.org \
--cc=penguin-kernel@I-love.SAKURA.ne.jp \
/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