linux-kernel - Re: [PATCH] sysfs: Remove page boundary align limitation on sysfs_emit and sysfs_emit

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:   Wed, 08 Sep 2021 22:44:23 -0700
From:   Joe Perches <joe@...ches.com>
To:     "Yu, Lang" <Lang.Yu@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J . Wysocki" <rafael@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] sysfs: Remove page boundary align limitation on
 sysfs_emit and sysfs_emit_at

On Thu, 2021-09-09 at 05:27 +0000, Yu, Lang wrote:
> [AMD Official Use Only]

this is a public list and this marker is not appropriate.


> > -----Original Message-----
> > From: Joe Perches <joe@...ches.com>
> > On Wed, 2021-09-08 at 20:07 +0800, Lang Yu wrote:
> > > The key purpose of sysfs_emit and sysfs_emit_at is to ensure that no
> > > overrun is done. Make them more equivalent with scnprintf.
> > 
> > I can't think of a single reason to do this.
> > sysfs_emit and sysfs_emit_at are specific to sysfs.
> > 
> > Use of these functions outside of sysfs is not desired or supported.
> > 
> Thanks for your reply. But I'm still curious why you put such a limitation.
> As "Documentation/filesystems/sysfs.rst" described, we can just  use 
> scnprintf(buf, PAGE_SIZE, "%s\n", dev->name) in show functions without
> such a limitation.

There's nothing particularly wrong with the use of scnprintf as above.

The only real reason that sysfs_emit exists is to be able to reduce the
kernel treewide quantity of uses of the sprintf family of functions
that need to be analyzed for possible buffer overruns.

The issue there is that buf is already known to be both a PAGE_SIZE buffer
and PAGE_SIZE aligned for sysfs show functions so there's no real reason
to use scnprintf.

sysfs_emit is a shorter/smaller function and using it could avoid some
sprintf defects.

> Some guys just try to replace scnprintf with sysfs_emit() or sysfs_emit_at() per above documents.

So don't do that.

> But sprintf and sysfs_emit/sysfs_emit_at are not totally equivalent(e.g., page boundary align).
> 
> In my opinion, we add a new api and try to replace an old api. Does we need to make it more
> compatible with old api?

IMO: no.