From: Omar Sandoval <osandov@osandov.com>
To: Jan Kara <jack@suse.cz>
Cc: Johannes Thumshirn <jthumshirn@suse.de>,
Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org
Subject: Re: [PATCH 0/15 v2] loop: Fix oops and possible deadlocks
Date: Tue, 16 Oct 2018 11:16:22 -0700 [thread overview]
Message-ID: <20181016181622.GF23044@vader> (raw)
In-Reply-To: <20181016113654.GG18918@quack2.suse.cz>
On Tue, Oct 16, 2018 at 01:36:54PM +0200, Jan Kara wrote:
> On Wed 10-10-18 14:28:09, Jan Kara wrote:
> > On Wed 10-10-18 13:42:27, Johannes Thumshirn wrote:
> > > On Wed, Oct 10, 2018 at 07:19:00PM +0900, Tetsuo Handa wrote:
> > > > On 2018/10/10 19:04, Jan Kara wrote:
> > > > > Hi,
> > > > >
> > > > > this patch series fixes oops and possible deadlocks as reported by syzbot [1]
> > > > > [2]. The second patch in the series (from Tetsuo) fixes the oops, the remaining
> > > > > patches are cleaning up the locking in the loop driver so that we can in the
> > > > > end reasonably easily switch to rereading partitions without holding mutex
> > > > > protecting the loop device.
> > > > >
> > > > > I have lightly tested the patches by creating, deleting, and modifying loop
> > > > > devices but if there's some more comprehensive loopback device testsuite, I
> > > > > can try running it. Review is welcome!
> > > >
> > > > Testing on linux-next by syzbot will be the most comprehensive. ;-)
> > >
> > > Apart from that blktests has a loop category and I think it could also be
> > > worthwhile to add the C reproducer from syzkaller to blktests.
> >
> > Yeah, I did run loop tests now and they ran fine. I can try converting the
> > syzbot reproducers into something legible but it will take a while.
>
> So I took a stab at this. But I hit two issues:
>
> 1) For the reproducer triggering the lockdep warning, you need a 32-bit
> binary (so that it uses compat_ioctl). I don't think we want to introduce
> 32-bit devel environment dependency to blktests. With 64-bits, the problem
> is also there but someone noticed and silenced lockdep (with a reason that
> I consider is incorrect)... I think the test is still worth it though as
> I'll remove the lockdep-fooling code in my patches and thus new breakage
> will be noticed.
Agreed, even if it doesn't trigger lockdep now, it's a good regression
test.
> 2) For the oops (use-after-free) issue I was not able to reproduce that in
> my test KVM in couple hours. The race window is rather narrow and syzbot
> with KASAN and everything hit it only 11 times. So I'm not sure how useful
> that test is. Any opinions?
I'd say we should add it anyways. If anything, it's a smoke test for
changing fds on a loop device. You could add a note that the race it's
testing for is very narrow.
next prev parent reply other threads:[~2018-10-16 18:16 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-10 10:04 [PATCH 0/15 v2] loop: Fix oops and possible deadlocks Jan Kara
2018-10-10 10:04 ` [PATCH 01/15] block/loop: Don't grab "struct file" for vfs_getattr() operation Jan Kara
2018-10-10 10:04 ` [PATCH 02/15] block/loop: Use global lock for ioctl() operation Jan Kara
2018-10-10 10:04 ` [PATCH 03/15] loop: Fold __loop_release into loop_release Jan Kara
2018-10-10 10:04 ` [PATCH 04/15] loop: Get rid of loop_index_mutex Jan Kara
2018-10-10 10:04 ` [PATCH 05/15] loop: Push lo_ctl_mutex down into individual ioctls Jan Kara
2018-10-10 10:04 ` [PATCH 06/15] loop: Split setting of lo_state from loop_clr_fd Jan Kara
2018-10-10 10:04 ` [PATCH 07/15] loop: Push loop_ctl_mutex down into loop_clr_fd() Jan Kara
2018-10-10 10:04 ` [PATCH 08/15] loop: Push loop_ctl_mutex down to loop_get_status() Jan Kara
2018-10-10 10:04 ` [PATCH 09/15] loop: Push loop_ctl_mutex down to loop_set_status() Jan Kara
2018-10-10 10:04 ` [PATCH 10/15] loop: Push loop_ctl_mutex down to loop_set_fd() Jan Kara
2018-10-10 10:04 ` [PATCH 11/15] loop: Push loop_ctl_mutex down to loop_change_fd() Jan Kara
2018-10-10 10:04 ` [PATCH 12/15] loop: Move special partition reread handling in loop_clr_fd() Jan Kara
2018-10-10 10:04 ` [PATCH 13/15] loop: Move loop_reread_partitions() out of loop_ctl_mutex Jan Kara
2018-10-10 10:04 ` [PATCH 14/15] loop: Fix deadlock when calling blkdev_reread_part() Jan Kara
2018-10-10 10:04 ` [PATCH 15/15] loop: Avoid circular locking dependency between loop_ctl_mutex and bd_mutex Jan Kara
2018-10-10 10:19 ` [PATCH 0/15 v2] loop: Fix oops and possible deadlocks Tetsuo Handa
2018-10-10 11:42 ` Johannes Thumshirn
2018-10-10 12:28 ` Jan Kara
2018-10-10 12:43 ` Johannes Thumshirn
2018-10-16 11:36 ` Jan Kara
2018-10-16 12:04 ` Johannes Thumshirn
2018-10-16 18:16 ` Omar Sandoval [this message]
2018-10-17 9:47 ` 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=20181016181622.GF23044@vader \
--to=osandov@osandov.com \
--cc=axboe@kernel.dk \
--cc=jack@suse.cz \
--cc=jthumshirn@suse.de \
--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.