All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Kara <jack@suse.cz>
To: Jens Axboe <axboe@kernel.dk>
Cc: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	<linux-block@vger.kernel.org>, Jan Kara <jack@suse.cz>
Subject: [PATCH] loop: Don't change loop device under exclusive opener
Date: Thu, 16 May 2019 16:01:27 +0200	[thread overview]
Message-ID: <20190516140127.23272-1-jack@suse.cz> (raw)

Loop module allows calling LOOP_SET_FD while there are other openers of
the loop device. Even exclusive ones. This can lead to weird
consequences such as kernel deadlocks like:

mount_bdev()				lo_ioctl()
  udf_fill_super()
    udf_load_vrs()
      sb_set_blocksize() - sets desired block size B
      udf_tread()
        sb_bread()
          __bread_gfp(bdev, block, B)
					  loop_set_fd()
					    set_blocksize()
            - now __getblk_slow() indefinitely loops because B != bdev
              block size

Fix the problem by disallowing LOOP_SET_FD ioctl when there are
exclusive openers of a loop device.

[Deliberately chosen not to CC stable as a user with priviledges to
trigger this race has other means of taking the system down and this
has a potential of breaking some weird userspace setup]

Reported-and-tested-by: syzbot+10007d66ca02b08f0e60@syzkaller.appspotmail.com
Signed-off-by: Jan Kara <jack@suse.cz>
---
 drivers/block/loop.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

Hi Jens!

What do you think about this patch? It fixes the problem but it also changes
user visible behavior so there are chances it breaks some existing setup
(although I have hard time coming up with a realistic scenario where it would
matter).

Alternatively we could change getblk() code handle changing block size. That
would fix the particular issue syzkaller found as well but I'm not sure what
else is broken when block device changes while fs driver is working with it.

Honza

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 102d79575895..f11b7dc16e9d 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -945,9 +945,20 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	if (!file)
 		goto out;
 
+	/*
+	 * If we don't hold exclusive handle for the device, upgrade to it
+	 * here to avoid changing device under exclusive owner.
+	 */
+	if (!(mode & FMODE_EXCL)) {
+		bdgrab(bdev);
+		error = blkdev_get(bdev, mode | FMODE_EXCL, loop_set_fd);
+		if (error)
+			goto out_putf;
+	}
+
 	error = mutex_lock_killable(&loop_ctl_mutex);
 	if (error)
-		goto out_putf;
+		goto out_bdev;
 
 	error = -EBUSY;
 	if (lo->lo_state != Lo_unbound)
@@ -1012,10 +1023,15 @@ static int loop_set_fd(struct loop_device *lo, fmode_t mode,
 	mutex_unlock(&loop_ctl_mutex);
 	if (partscan)
 		loop_reread_partitions(lo, bdev);
+	if (!(mode & FMODE_EXCL))
+		blkdev_put(bdev, mode | FMODE_EXCL);
 	return 0;
 
 out_unlock:
 	mutex_unlock(&loop_ctl_mutex);
+out_bdev:
+	if (!(mode & FMODE_EXCL))
+		blkdev_put(bdev, mode | FMODE_EXCL);
 out_putf:
 	fput(file);
 out:
-- 
2.16.4


             reply	other threads:[~2019-05-16 14:01 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-16 14:01 Jan Kara [this message]
2019-05-16 20:44 ` [PATCH] loop: Don't change loop device under exclusive opener Jens Axboe
2019-05-27 12:29   ` Jan Kara
2019-05-27 13:34     ` Jens Axboe
2019-07-18  8:15       ` Kai-Heng Feng
2019-07-30  9:29         ` Jan Kara
2019-07-30  9:36           ` John Lenton
2019-07-30 10:16             ` Jan Kara
2019-07-30 13:36               ` Jan Kara
2019-07-30 17:59                 ` Kai-Heng Feng
2019-07-30 19:17                 ` Jens Axboe
2019-07-30 21:11                   ` John Lenton
2019-07-31  8:56                   ` Jan Kara
2019-08-05 16:41                 ` Bart Van Assche
2019-08-05 21:01                   ` Tetsuo Handa
2019-08-07  9:45                   ` Jan Kara
2019-08-07 18:45                     ` Bart Van Assche
2019-08-08 13:37                     ` Jens Axboe
2019-07-30 10:16           ` 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=20190516140127.23272-1-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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.