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

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

[Thomas Maier]

[-- Attachment #1: Type: text/plain, Size: 474 bytes --]

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 <balagi@justmail.de>

[-- Attachment #2: sysfs_write_file-zero-term-string.patch --]
[-- Type: application/octet-stream, Size: 611 bytes --]

diff -urpN linux-2.6.19-rc2-mm2.sysfs/fs/sysfs/file.c 2-sysfs_write_file-string/fs/sysfs/file.c
--- linux-2.6.19-rc2-mm2.sysfs/fs/sysfs/file.c	2006-10-22 18:38:47.000000000 +0200
+++ 2-sysfs_write_file-string/fs/sysfs/file.c	2006-10-22 18:45:38.000000000 +0200
@@ -195,6 +195,9 @@ fill_write_buffer(struct sysfs_buffer * 
 		count = PAGE_SIZE - 1;
 	error = copy_from_user(buffer->page,buf,count);
 	buffer->needs_read_fill = 1;
+	/* if buf is assumed to contain a string, terminate it by \0,
+	   so e.g. sscanf() can scan the string easily */
+	buffer->page[count] = 0;
 	return error ? -EFAULT : count;
 }
 

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

[Greg KH]

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

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

[Thomas Maier]

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 <gregkh@suse.de>:

> 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
>
>

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

[Greg KH]

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

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

[balagi]

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