Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] loop: Don't change loop device under exclusive opener
@ 2019-05-16 14:01 Jan Kara
  2019-05-16 20:44 ` Jens Axboe
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Kara @ 2019-05-16 14:01 UTC (permalink / raw)
  To: Jens Axboe; +Cc: Tetsuo Handa, linux-block, Jan Kara

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


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, back to index

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-16 14:01 [PATCH] loop: Don't change loop device under exclusive opener Jan Kara
2019-05-16 20:44 ` 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

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org
	public-inbox-index linux-block

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git