From: Jan Kara <jack@suse.cz>
To: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc: Jan Kara <jack@suse.cz>, Jens Axboe <axboe@kernel.dk>,
Alexander Viro <viro@zeniv.linux.org.uk>,
syzbot <syzbot+10007d66ca02b08f0e60@syzkaller.appspotmail.com>,
dvyukov@google.com, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com,
linux-block@vger.kernel.org
Subject: Re: INFO: task hung in __get_super
Date: Thu, 16 May 2019 13:48:17 +0200 [thread overview]
Message-ID: <20190516114817.GD13274@quack2.suse.cz> (raw)
In-Reply-To: <20190515130730.GA9526@quack2.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]
On Wed 15-05-19 15:07:30, Jan Kara wrote:
> On Wed 15-05-19 20:32:27, Tetsuo Handa wrote:
> > On 2019/05/15 19:21, Jan Kara wrote:
> > > The question is how to fix this problem. The simplest fix I can see is that
> > > we'd just refuse to do LOOP_SET_FD if someone has the block device
> > > exclusively open as there are high chances such user will be unpleasantly
> > > surprised by the device changing under him. OTOH this has some potential
> > > for userspace visible regressions. But I guess it's worth a try. Something
> > > like attached patch?
> >
> > (1) If I understand correctly, FMODE_EXCL is set at blkdev_open() only if
> > O_EXCL is specified.
>
> Yes.
>
> > How can we detect if O_EXCL was not used, for the reproducer (
> > https://syzkaller.appspot.com/text?tag=ReproC&x=135385a8a00000 ) is not
> > using O_EXCL ?
>
> mount_bdev() is using O_EXCL and that's what matters.
>
> > (2) There seems to be no serialization. What guarantees that mount_bdev()
> > does not start due to preempted after the check added by this patch?
>
> That's a good question. lo_ctl_mutex actually synchronizes most of this
> (taken in both loop_set_fd() and lo_open()) but you're right that there's
> still a small race window where loop_set_fd() need not see bdev->bd_holders
> elevated while blkdev_get() will succeed. So I need to think a bit more
> about proper synchronization of this.
OK, so non-racy fix was a bit more involved and I've ended up just
upgrading the file reference to an exclusive one in loop_set_fd() instead
of trying to hand-craft some locking solution. The result is attached and
it passes blktests.
Let syzbot also test it:
#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git v5.1
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
[-- Attachment #2: 0001-loop-Don-t-change-loop-device-under-exclusive-opener.patch --]
[-- Type: text/x-patch, Size: 2344 bytes --]
From e7a35f48a902b42894eba8cc4201ca745bfd5dfe Mon Sep 17 00:00:00 2001
From: Jan Kara <jack@suse.cz>
Date: Wed, 15 May 2019 11:45:10 +0200
Subject: [PATCH] loop: Don't change loop device under exclusive opener
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-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(-)
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
next prev parent reply other threads:[~2019-05-16 11:48 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <0000000000002cd22305879b22c4@google.com>
[not found] ` <201905150102.x4F12b6o009249@www262.sakura.ne.jp>
2019-05-15 10:21 ` INFO: task hung in __get_super Jan Kara
2019-05-15 11:32 ` Tetsuo Handa
2019-05-15 13:07 ` Jan Kara
2019-05-16 11:48 ` Jan Kara [this message]
2019-05-16 12:17 ` Tetsuo Handa
2019-05-16 12:32 ` Jan Kara
2019-05-16 12:50 ` Tetsuo Handa
2019-05-16 12:33 ` syzbot
2019-05-15 11:59 ` syzbot
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=20190516114817.GD13274@quack2.suse.cz \
--to=jack@suse.cz \
--cc=axboe@kernel.dk \
--cc=dvyukov@google.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=penguin-kernel@i-love.sakura.ne.jp \
--cc=syzbot+10007d66ca02b08f0e60@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=viro@zeniv.linux.org.uk \
/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).