lkml.org 
[lkml]   [2004]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] Possible race in sysfs_read_file() and sysfs_write_file()

Hello,

I think there is a possibility for two threads from a single process to
race in sysfs_read_file() if they call read() on the same file at the same
time.

Supposing no read() has been done on this file previously, the two threads
could end allocating each a new page in fill_read_buffer() for
buffer->page, and one of these pages would be lost forever.

The same applies to write().

This patch adds a semaphore in struct sysfs_buffer to fix this issue.

Now it should be sufficient, to fix this memory leak, to protect only the
call to get_zeroed_page() in fill_read_buffer() with the semaphore.

Instead, this patch holds the semaphore during the whole
sysfs_read_file().

The reason is that there is also a risk, with the current code, of having
one thread in flush_read_buffer() while the another is in
fill_read_buffer()/ops->show(), and then maybe read() returning garbage.

This is more likely to happen actually if two threads call write() at the
same time with different data, with one thread using the data of the
buffer->page in ops->store(), and another thread overwriting that data in
fill_write_buffer(), thus maybe having ops->store() using corrupted data.

Of course in this case, the multithreaded application is broken and should
not expect a correct kernel behaviour... so maybe only protection around
get_zeroed_page() is needed.

Simon.

This patch is against 2.6.9-rc1-mm2.


Signed-off-by: Simon Derr <simon.derr@bull.net>

Index: 269mm2kdb/fs/sysfs/file.c
===================================================================
--- 269mm2kdb.orig/fs/sysfs/file.c 2004-08-31 11:29:16.000000000 +0200
+++ 269mm2kdb/fs/sysfs/file.c 2004-09-01 10:00:57.582251616 +0200
@@ -6,6 +6,7 @@
#include <linux/dnotify.h>
#include <linux/kobject.h>
#include <asm/uaccess.h>
+#include <asm/semaphore.h>

#include "sysfs.h"

@@ -53,6 +54,7 @@
loff_t pos;
char * page;
struct sysfs_ops * ops;
+ struct semaphore sem;
};


@@ -140,13 +142,17 @@
struct sysfs_buffer * buffer = file->private_data;
ssize_t retval = 0;

+ down(&buffer->sem);
if (!*ppos) {
if ((retval = fill_read_buffer(file->f_dentry,buffer)))
- return retval;
+ goto out;
}
pr_debug("%s: count = %d, ppos = %lld, buf = %s\n",
__FUNCTION__,count,*ppos,buffer->page);
- return flush_read_buffer(buffer,buf,count,ppos);
+ retval = flush_read_buffer(buffer,buf,count,ppos);
+out:
+ up(&buffer->sem);
+ return retval;
}


@@ -220,11 +226,13 @@
{
struct sysfs_buffer * buffer = file->private_data;

+ down(&buffer->sem);
count = fill_write_buffer(buffer,buf,count);
if (count > 0)
count = flush_write_buffer(file->f_dentry,buffer,count);
if (count > 0)
*ppos += count;
+ up(&buffer->sem);
return count;
}

@@ -287,6 +295,7 @@
buffer = kmalloc(sizeof(struct sysfs_buffer),GFP_KERNEL);
if (buffer) {
memset(buffer,0,sizeof(struct sysfs_buffer));
+ init_MUTEX(&buffer->sem);
buffer->ops = ops;
file->private_data = buffer;
} else
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
\
 
 \ /
  Last update: 2005-03-22 14:05    [W:0.063 / U:0.340 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site