linux-kernel - Re: [PATCH] leds: remove PAGE_SIZE limit of /sys/class/leds/<led>/trigger

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: <20190902181207.GA18577@kroah.com>
Date:   Mon, 2 Sep 2019 20:12:07 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jacek Anaszewski <jacek.anaszewski@...il.com>
Cc:     Akinobu Mita <akinobu.mita@...il.com>, linux-leds@...r.kernel.org,
        linux-kernel@...r.kernel.org, Pavel Machek <pavel@....cz>,
        Dan Murphy <dmurphy@...com>
Subject: Re: [PATCH] leds: remove PAGE_SIZE limit of
 /sys/class/leds/<led>/trigger

On Sun, Sep 01, 2019 at 06:53:34PM +0200, Jacek Anaszewski wrote:
> Hi Akinobu,
> 
> Thank you for the patch.
> 
> I have one nit below but in general it looks good to me.
> I've tested it with 2000 mtd triggers (~14kB file size)
> and it worked flawlessly.
> 
> Still, I would like to have ack from Greg for it.
> 
> Adding Greg on Cc.
> 
> On 8/29/19 4:49 PM, Akinobu Mita wrote:
> > Reading /sys/class/leds/<led>/trigger returns all available LED triggers.
> > However, the size of this file is limited to PAGE_SIZE because of the
> > limitation for sysfs attribute.
> > 
> > Enabling LED CPU trigger on systems with thousands of CPUs easily hits
> > PAGE_SIZE limit, and makes it impossible to see all available LED triggers
> > and which trigger is currently activated.
> > 
> > This converts /sys/class/leds/<led>/trigger to bin attribute and removes
> > the PAGE_SIZE limitation.

But this is NOT a binary file.  A sysfs binary file is used for when the
kernel passes data to or from hardware without any parsing of the data
by the kernel.

You are not doing that here, you are abusing the "one value per file"
rule of sysfs so much that you are forced to work around the limitation
it put in place on purpose to keep you from doing stuff like this.

Please fix this "correctly" by creating a new api that works properly
and just live with the fact that this file will never work correctly and
move everyone to use the new api instead.

Don't keep on abusing the interface by workarounds like this, it is not
ok.

thanks,

greg k-h