public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
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 15/16] loop: Avoid circular locking dependency between loop_ctl_mutex and bd_mutex
Date: Thu,  8 Nov 2018 14:01:15 +0100	[thread overview]
Message-ID: <20181108130116.12140-16-jack@suse.cz> (raw)
In-Reply-To: <20181108130116.12140-1-jack@suse.cz>

Code in loop_change_fd() drops reference to the old file (and also the
new file in a failure case) under loop_ctl_mutex. Similarly to a
situation in loop_set_fd() this can create a circular locking dependency
if this was the last reference holding the file open. Delay dropping of
the file reference until we have released loop_ctl_mutex.

Reported-by: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/block/loop.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index b3f981ac8ef1..112afc9bc604 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -677,7 +677,7 @@ static int loop_validate_file(struct file *file, struct block_device *bdev)
 static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 			  unsigned int arg)
 {
-	struct file	*file, *old_file;
+	struct file	*file = NULL, *old_file;
 	int		error;
 	bool		partscan;
 
@@ -686,21 +686,21 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 		return error;
 	error = -ENXIO;
 	if (lo->lo_state != Lo_bound)
-		goto out_unlock;
+		goto out_err;
 
 	/* the loop device has to be read-only */
 	error = -EINVAL;
 	if (!(lo->lo_flags & LO_FLAGS_READ_ONLY))
-		goto out_unlock;
+		goto out_err;
 
 	error = -EBADF;
 	file = fget(arg);
 	if (!file)
-		goto out_unlock;
+		goto out_err;
 
 	error = loop_validate_file(file, bdev);
 	if (error)
-		goto out_putf;
+		goto out_err;
 
 	old_file = lo->lo_backing_file;
 
@@ -708,7 +708,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 
 	/* size of the new backing store needs to be the same */
 	if (get_loop_size(lo, file) != get_loop_size(lo, old_file))
-		goto out_putf;
+		goto out_err;
 
 	/* and ... switch */
 	blk_mq_freeze_queue(lo->lo_queue);
@@ -719,18 +719,22 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 			     lo->old_gfp_mask & ~(__GFP_IO|__GFP_FS));
 	loop_update_dio(lo);
 	blk_mq_unfreeze_queue(lo->lo_queue);
-
-	fput(old_file);
 	partscan = lo->lo_flags & LO_FLAGS_PARTSCAN;
 	mutex_unlock(&loop_ctl_mutex);
+	/*
+	 * We must drop file reference outside of loop_ctl_mutex as dropping
+	 * the file ref can take bd_mutex which creates circular locking
+	 * dependency.
+	 */
+	fput(old_file);
 	if (partscan)
 		loop_reread_partitions(lo, bdev);
 	return 0;
 
-out_putf:
-	fput(file);
-out_unlock:
+out_err:
 	mutex_unlock(&loop_ctl_mutex);
+	if (file)
+		fput(file);
 	return error;
 }
 
-- 
2.16.4

  parent reply	other threads:[~2018-11-08 13:01 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 ` [PATCH 13/16] loop: Move loop_reread_partitions() out of loop_ctl_mutex Jan Kara
2018-11-08 13:01 ` [PATCH 14/16] loop: Fix deadlock when calling blkdev_reread_part() Jan Kara
2018-11-08 13:01 ` Jan Kara [this message]
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-16-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