linux-kernel - Re: [PATCH v7 09/12] sysfs: fix deadlock race with module removal

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: <YUkAkSUQtUZvNvTu@bombadil.infradead.org>
Date:   Mon, 20 Sep 2021 14:43:45 -0700
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Bart Van Assche <bvanassche@....org>
Cc:     tj@...nel.org, gregkh@...uxfoundation.org,
        akpm@...ux-foundation.org, minchan@...nel.org, jeyu@...nel.org,
        shuah@...nel.org, rdunlap@...radead.org, rafael@...nel.org,
        masahiroy@...nel.org, ndesaulniers@...gle.com, yzaikin@...gle.com,
        nathan@...nel.org, ojeda@...nel.org,
        penguin-kernel@...ove.sakura.ne.jp, vitor@...saru.org,
        elver@...gle.com, jarkko@...nel.org, glider@...gle.com,
        rf@...nsource.cirrus.com, stephen@...workplumber.org,
        David.Laight@...lab.com, jolsa@...nel.org,
        andriy.shevchenko@...ux.intel.com, trishalfonso@...gle.com,
        andreyknvl@...il.com, jikos@...nel.org, mbenes@...e.com,
        ngupta@...are.org, sergey.senozhatsky.work@...il.com,
        reinette.chatre@...el.com, fenghua.yu@...el.com, bp@...en8.de,
        x86@...nel.org, hpa@...or.com, lizefan.x@...edance.com,
        hannes@...xchg.org, daniel.vetter@...ll.ch, bhelgaas@...gle.com,
        kw@...ux.com, dan.j.williams@...el.com, senozhatsky@...omium.org,
        hch@....de, joe@...ches.com, hkallweit1@...il.com, axboe@...nel.dk,
        jpoimboe@...hat.com, tglx@...utronix.de, keescook@...omium.org,
        rostedt@...dmis.org, peterz@...radead.org,
        linux-spdx@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-block@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kselftest@...r.kernel.org, cgroups@...r.kernel.org,
        linux-kernel@...r.kernel.org, copyleft-next@...ts.fedorahosted.org
Subject: Re: [PATCH v7 09/12] sysfs: fix deadlock race with module removal

On Mon, Sep 20, 2021 at 02:36:38PM -0700, Bart Van Assche wrote:
> On 9/17/21 10:04 PM, Luis Chamberlain wrote:
> > A sketch of how this can happen follows:
> > 
> > CPU A                              CPU B
> >                                     whatever_store()
> > module_unload
> >    mutex_lock(foo)
> >                                     mutex_lock(foo)
> >     del_gendisk(zram->disk);
> >       device_del()
> >         device_remove_groups()
> > 
> > In this situation whatever_store() is waiting for the mutex foo to
> > become unlocked, but that won't happen until module removal is complete.
> > But module removal won't complete until the sysfs file being poked
> > completes which is waiting for a lock already held.
> 
> If I remember correctly I encountered the deadlock scenario described
> above for the first time about ten years ago while working on the SCST
> project. We solved this deadlock by removing the sysfs attributes from
> the module unload code before grabbing mutex_lock(foo), e.g. by calling
> sysfs_remove_file().

Well the sysfs attributes in zram do tons of funky mucking around so
unfortunately no. It's not the only driver where this can happen. It is
why I decided to work on a generic solution instead.

> This works because calling sysfs_remove_file()
> multiple times in a row is safe. Is that solution good enough for the
> zram driver?

The sysfs attributes are group attributes part of the block, and so are
removed for the driver on a del_gendisk(). So unfortunately no, this
would not be a good solution in this case.

  Luis