Re: [PATCH RESEND v2 00/30] proc: convert the fs, kernel and mm entries (Part 5/7)

[David Laight]

On Fri, 6 Nov 2026 12:10:13 -0800
Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

> Next comment:  3c7a6276d211 ("scsi: core: NUL-terminate what is
> written to the host proc files") is nasty.

I've been looking at the similar sysfs code.
Most of the 'driver' functions are passed a char[PAGE_SIZE] buffer and
use sysfs_emit() to add text.
They are expected to add the trailing '\n' - even though it is pretty
much mandatory.
There is also a lot of code that loops:
	offset += sysfs_emit_at(buf, offset, "%s, ", xxx);
and then has to go through horrid hoops to get the line terminated.

But the outer wrapper code can do it easily.
It has the buffer and length so can 'fiddle' with the end before telling
the seq_buf code how much data is in the buffer.

I managed to build a kernel where the buf parameter to sysfs_emit() is
a pointer to 'typedef struct { char c; } sysfs_buf'.
Pretty much all of the changes just changed the name of the type or
replaced printf() with sysfs_emit().
Even with just 'typdef char sysfs_buf' it would be more obvious when
sysfs_emit() was valid.

Don't look in the amdgpu driver it does:
	off = buf & (PAGE - 1);
	buf -= off;
	sysfs_emit_at(buf, off, ....)
AFAICT it has proper aligned buffers - or my build would have failed.

I did find one disk related driver that copied (probably) 512 bytes
of hex data into the buffer.
That is nasty, normal sysfs has the option of raw read/write, but they
get lost by the time you are in the dev_xxx() functions.

OTOH the outer 'attr' structure has some padding (even on 32bit)
so there is scope for some flags.
(There is one flag, but it gets set in the file permissions word
which is all rather nasty.)

Then there is some of the raw code, I didn't follow it all through
but it really looks like the user supplied length isn't checked
properly.
For sub 4k buffers using the same code paths as text might make sense.

There is also the pre-alloc code added for some entries to avoid kmalloc()
during reads by allocating a buffer during open.
Looking carefully the normal buffer is allocated during the first read
but not freed until close - so it wouldn't be very different to just
allocate it a little earlier, removing some other little-used code paths.

David