2006-10-22 17:15:29

by Thomas Maier

[permalink] [raw]
Subject: [PATCH] 2.6.19-rc2-mm2 sysfs: sysfs_write_file() writes zero terminated data

Hello,

since most of the files in sysfs are text files,
it would be nice, if the "store" function called
during sysfs_write_file() gets a zero terminated
string / data.
The current implementation seems not to ensure this.
(But only if it is the first time the zeroed buffer
page is allocated.)

So the buffer can be scanned by sscanf() easily,
for example.

This patch simply sets a \0 char behind the
data in buffer->page.

Signed-off-by: Thomas Maier <[email protected]>


Attachments:
sysfs_write_file-zero-term-string.patch (611.00 B)

2006-10-22 18:39:34

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] 2.6.19-rc2-mm2 sysfs: sysfs_write_file() writes zero terminated data

On Sun, Oct 22, 2006 at 07:17:47PM +0200, Thomas Maier wrote:
> Hello,
>
> since most of the files in sysfs are text files,
> it would be nice, if the "store" function called
> during sysfs_write_file() gets a zero terminated
> string / data.
> The current implementation seems not to ensure this.
> (But only if it is the first time the zeroed buffer
> page is allocated.)

Have you seen sysfs buffers being passed to the store() function in a
non-null terminated manner? How?

Are you seeking backward and then writing again to the file somehow?

thanks,

greg k-h

2006-10-23 19:59:42

by Thomas Maier

[permalink] [raw]
Subject: Re: [PATCH] 2.6.19-rc2-mm2 sysfs: sysfs_write_file() writes zero terminated data

Hello,

Sorry, maybe i missed something, but according to the
code in fs/sysfs/file.c the "write" sequence is:

- call to sysfs_write_file(ubuf, count)
- if (!sysfsbuf->page) alloc zeroed page
- copy count bytes from ubuf to sysfsbuf->page
- call store(sysfsbuf->page, count)

When you write again to the file before closing it
(possible?!), and count is less the the previous count
you may not pass a zero terminated string/data to store().

-Thomas


Am 22.10.2006, 20:39 Uhr, schrieb Greg KH <[email protected]>:

> On Sun, Oct 22, 2006 at 07:17:47PM +0200, Thomas Maier wrote:
>> Hello,
>>
>> since most of the files in sysfs are text files,
>> it would be nice, if the "store" function called
>> during sysfs_write_file() gets a zero terminated
>> string / data.
>> The current implementation seems not to ensure this.
>> (But only if it is the first time the zeroed buffer
>> page is allocated.)
>
> Have you seen sysfs buffers being passed to the store() function in a
> non-null terminated manner? How?
>
> Are you seeking backward and then writing again to the file somehow?
>
> thanks,
>
> greg k-h
>
>


2006-10-23 21:18:35

by Greg KH

[permalink] [raw]
Subject: Re: [PATCH] 2.6.19-rc2-mm2 sysfs: sysfs_write_file() writes zero terminated data

On Mon, Oct 23, 2006 at 10:02:03PM +0200, Thomas Maier wrote:
> Hello,
>
> Sorry, maybe i missed something, but according to the
> code in fs/sysfs/file.c the "write" sequence is:
>
> - call to sysfs_write_file(ubuf, count)
> - if (!sysfsbuf->page) alloc zeroed page
> - copy count bytes from ubuf to sysfsbuf->page
> - call store(sysfsbuf->page, count)
>
> When you write again to the file before closing it
> (possible?!), and count is less the the previous count
> you may not pass a zero terminated string/data to store().

Yeah, that might happen, but writing to a sysfs file again after the
first time is not the normal case here. I'll add your patch to the
queue to keep this from happening though, good catch.

greg k-h

2006-10-26 12:08:29

by Thomas Maier

[permalink] [raw]
Subject: Re: [PATCH] 2.6.19-rc2-mm2 sysfs: sysfs_write_file() writes zero terminated data

Hello,

> Yeah, that might happen, but writing to a sysfs file again after the
> first time is not the normal case here. I'll add your patch to the
> queue to keep this from happening though, good catch.

If the patch is applied, the get_zeroed_page() call can be replaced
by __get_free_pages() to save some cpu time.

-Thomas