linux-kernel - Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug multistate

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]

Date:   Thu, 8 Apr 2021 10:55:53 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jiri Kosina <jikos@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Luis Chamberlain <mcgrof@...nel.org>,
        Minchan Kim <minchan@...nel.org>, keescook@...omium.org,
        dhowells@...hat.com, hch@...radead.org, mbenes@...e.com,
        ngupta@...are.org, sergey.senozhatsky.work@...il.com,
        Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] zram: fix crashes due to use of cpu hotplug
 multistate

On Thu, Apr 08, 2021 at 10:35:17AM +0200, Jiri Kosina wrote:
> On Thu, 8 Apr 2021, Greg KH wrote:
> 
> > > If there is a driver/subsystem code that can't handle the reverse 
> > > operation to modprobe, it clearly can't handle error handling during 
> > > modprobe (which, one would hope, is supported), and should be fixed.
> > 
> > Huh?  No, that's not the issue here, it's the issue of different
> > userspace code paths into the module at the same time that it is trying
> > to be unloaded.  That has nothing to do with loading the module the
> > first time as userspace is not touching those apis yet.
> 
> So do you claim that once the first (out of possibly many) 
> userspace-visible sysfs entry has been created during module insertion and 
> made available to userspace, there is never going to be rollback happening 
> that'd be removing that first sysfs entry again?

{sigh}

I'm not trying to argue that, no.

What I am arguing is that the complexity that the original patch was
not worth the low probablity of this actually being an issue hit in
real-life operations.

That's all, messing around with sysfs entries and module reference
counts is tricky and complex and a total mess.  We have a separation
between normal sysfs files and devices being removed that should handle
the normal operations but there are still some crazy corner cases, of
which this seems to be one.

Module removal is not a "normal" operation that can be triggered by a
system automatically without a user asking for it.  As someone reminded
me on IRC, we used to do this "automatically" for many problematic
drivers years ago for suspend/resume, that should all now be long fixed
up.

So to add crazy complexity to the kernel, for an operation that can only
be triggered manually by a root user, is not worth it in my opinion, as
the maintainer of that code the complexity was asked to be made to.

My throw-away comment of "module unloading is not supported" was an
attempt to summarize all of the above into one single sentence that
seems to have struck a nerve with a lot of people, and I appologize for
that :(

thanks,

greg k-h