linux-kernel - Re: [PATCH] bdi: Fix another oops in wb

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20180613143315.GS1351649@devbig577.frc2.facebook.com>
Date:   Wed, 13 Jun 2018 07:33:15 -0700
From:   Tejun Heo <tj@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Jens Axboe <axboe@...nel.dk>,
        syzbot <syzbot+4a7438e774b21ddd8eca@...kaller.appspotmail.com>,
        syzkaller-bugs <syzkaller-bugs@...glegroups.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Al Viro <viro@...iv.linux.org.uk>,
        Dave Chinner <david@...morbit.com>,
        linux-block@...r.kernel.org,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] bdi: Fix another oops in wb_workfn()

Hello, Jan.

On Tue, Jun 12, 2018 at 05:57:54PM +0200, Jan Kara wrote:
> > Yeah, right, so the root cause is that we're walking the wb_list while
> > holding lock and expecting the object to stay there even after lock is
> > released.  Hmm... we can use a mutex to synchronize the two
> > destruction paths.  It's not like they're hot paths anyway.
> 
> Hmm, do you mean like having a per-bdi or even a global mutex that would
> protect whole wb_shutdown()? Yes, that should work and we could get rid of
> WB_shutting_down bit as well with that. Just it seems a bit strange to

Yeap.

> introduce a mutex only to synchronize these two shutdown paths - usually
> locks protect data structures and in this case we have cgwb_lock for
> that so it looks like a duplication from a first look.

Yeah, I feel a bit reluctant too but I think that's the right thing to
do here.  This is an inherently weird case where there are two ways
that an object can go away with the immediate drain requirement from
one side.  It's not a hot path and the dumber the synchronization the
better, right?

Thanks.

-- 
tejun