Re: [PATCH] sysfs: clamp show() return value in sysfs_kf_read()

[David Laight <david.laight.linux@gmail.com>]

On Thu, 21 May 2026 08:19:16 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Wed, May 20, 2026 at 11:11:58PM +0100, David Laight wrote:
> > On Wed, 20 May 2026 15:07:01 +0200
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> >   
> > > sysfs_kf_seq_show() defends against buggy show() callbacks that return
> > > larger than PAGE_SIZE by clamping the value and printing a warning.
> > > sysfs_kf_read(), the prealloc variant, has no such defense.
> > > 
> > > The only current in-tree user of __ATTR_PREALLOC is drivers/md/md.c,
> > > whose show() callbacks are well-behaved, so this is hardening against
> > > future drivers doing foolish things and out-of-tree code doing even more
> > > foolish things.  
> > 
> > What is the rational for it using PREALLOC?  
> 
> No idea, you might want to dig to find the commit that did this.

This one:
commit 750f199ee8b578062341e6ddfe36c59ac8ff2dcb
Author: NeilBrown <neil@brown.name>
Date:   Tue Sep 30 08:53:05 2014 +1000

    md: mark some attributes as pre-alloc
    
    Since __ATTR_PREALLOC was introduced in v3.19-rc1~78^2~18
    it can now be used by md.
    
    This ensure that writing to these sysfs attributes will never
    block due to a memory allocation.
    Such blocking could become a deadlock if mdmon is trying to
    reconfigure an array after a failure prior to re-enabling writes.

That might be better handled with a flag that changes the kmalloc()
to NOIO and falls back onto a global preallocated page in the
normal path.
It would certainly let a lot of code be deleted (always good).

The atomic_write_len would then just be a max_write_len
(and maybe renamed).
The only real use is the 'cgroup' code that wants to support writes
that are larger than 4k.

Currently, if atomic_write_len is zero writes are truncated to PAGE_SIZE,
if non-zero overlong writes are rejected.

AFAICT the file offset is never checked - so all writes are (sort of) appends.
I'm not at all sure that makes sense.
Possibly all overlong writes should be rejected.

-- David

> 
> thanks,
> 
> greg k-h