linux-kernel - Re: [PATCH] usb: atm: don't use snprintf() for sysfs attrs

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: <20200825082406.GB1335351@kroah.com>
Date:   Tue, 25 Aug 2020 10:24:06 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     David Laight <David.Laight@...lab.com>
Cc:     'Alex Dewar' <alex.dewar90@...il.com>,
        "Gustavo A. R. Silva" <gustavoars@...nel.org>,
        "accessrunner-general@...ts.sourceforge.net" 
        <accessrunner-general@...ts.sourceforge.net>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: atm: don't use snprintf() for sysfs attrs

On Tue, Aug 25, 2020 at 08:12:05AM +0000, David Laight wrote:
> From: Alex Dewar
> > Sent: 24 August 2020 23:23
> > kernel/cpu.c: don't use snprintf() for sysfs attrs
> > 
> > As per the documentation (Documentation/filesystems/sysfs.rst),
> > snprintf() should not be used for formatting values returned by sysfs.
> > 
> > In all of these cases, sprintf() suffices as we know that the formatted
> > strings will be less than PAGE_SIZE in length.
> 
> Hmmmm....
> I much prefer to see bounded string ops.
> sysfs really ought to be passing through the buffer length.

No.

> The buffer size should probably be SYSFS_BUF_LEN not PAGE_SIZE
> (even it happens to typically be the same).

Nope.

> If PAGE_SIZE is big (or small) passing a 4k buffer may be
> more appropriate than a PAGE_SIZE one.

Nope, sysfs is supposed to be "one value per file", and the buffer size
was specifically not passed in because you should _NEVER_ need to care
about it because all you are doing is printing out a single value.

If you really want to print more than just a single value, you should
not use sysfs.

So this is designed this way on purpose, you shouldn't have to worry
about any of this, and that way, you don't have to "program
defensively", it all just works in a simple manner.

thanks,

greg k-h