linux-kernel - Re: [PATCH] zram: restrict add/remove attributes to root only

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:   Sun, 4 Dec 2016 12:28:20 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Sergey Senozhatsky <sergey.senozhatsky@...il.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Minchan Kim <minchan@...nel.org>,
        Steven Allen <steven@...balien.com>,
        linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky.work@...il.com>
Subject: Re: [PATCH] zram: restrict add/remove attributes to root only

On Sun, Dec 04, 2016 at 07:52:08PM +0900, Sergey Senozhatsky wrote:
> On (12/04/16 11:28), Greg KH wrote:
> > On Sun, Dec 04, 2016 at 11:35:15AM +0900, Sergey Senozhatsky wrote:
> [..]
> 
> > Why can't a normal user read the attribute?  Does a read actually modify
> > something?
> 
> yes, it does.

Oh that's totally and completely broken then.

Reading from a sysfs file should NEVER cause side affects to the system.
Please fix up this api.

> reading from a hot_add file creates a new zram device and returns a new
> device's device_id. not initialized device (so it does not eat the memory
> for handle table, etc.), but with its own set of sysfs attrs, etc. which
> consumes memory after all. so a 'normal' user, doing a simple read from a
> hot_add file in a loop just for fun, can create a lot of devices and,
> quite likely, cause some troubles (as reported by Steven Allen).

Please switch this to be a char device node if you wish to "write and
get a device handle back".  I don't know how I missed that in the
original api review, sorry about that.

For now, you need to document the heck out of this in the attribute
declaration that this is what is going on.  Otherwise someone like me
will come along and "fix up" the file to use ATTR_RO again in the
future and you will have the same problem again.

thanks,

greg k-h