[PATCH] sysfs write fixes

[PATCH] sysfs write fixes

[Williams, Mitch A]

This patch corrects some errors that we saw while writing to sysfs
files.
 - Attempts to open the file in append mode result in error
 - Writes are buffered and flushed to the kobject owner during close
 - Attempts to seek on sysfs files result in error

Generated from 2.6.11-rc1.

It's not a big patch, but if you'd like it whacked up into smaller ones,
I'll
be glad to do so.

Signed-off-by:  Mitch Williams <mitch.a.williams@intel.com>


diff -uprN -X dontdiff linux-2.6.11-clean/fs/sysfs/file.c
linux-2.6.11/fs/sysfs/file.c
--- linux-2.6.11-clean/fs/sysfs/file.c	2004-12-24 13:33:50.000000000
-0800
+++ linux-2.6.11/fs/sysfs/file.c	2005-01-21 13:09:21.000000000
-0800
@@ -1,5 +1,11 @@
 /*
  * file.c - operations for regular (text) files.
+ *
+ * Changes:
+ * 2004/01/21 Mitch Williams <mitch.a.williams at intel.com>
+ *   - Changed llseek method to be no_llseek.
+ *   - Opening a file for append now returns an error.
+ *   - Writes are now buffered and flushed at close
  */
 
 #include <linux/module.h>
@@ -55,7 +61,8 @@ struct sysfs_buffer {
 	char			* page;
 	struct sysfs_ops	* ops;
 	struct semaphore	sem;
-	int			needs_read_fill;
+	int			read_filled;
+	int			needs_write_flush;
 };
 
 
@@ -83,7 +90,7 @@ static int fill_read_buffer(struct dentr
 		return -ENOMEM;
 
 	count = ops->show(kobj,attr,buffer->page);
-	buffer->needs_read_fill = 0;
+	buffer->read_filled = 1;
 	BUG_ON(count > (ssize_t)PAGE_SIZE);
 	if (count >= 0)
 		buffer->count = count;
@@ -148,7 +155,7 @@ sysfs_read_file(struct file *file, char 
 	ssize_t retval = 0;
 
 	down(&buffer->sem);
-	if (buffer->needs_read_fill) {
+	if (!buffer->read_filled) {
 		if ((retval = fill_read_buffer(file->f_dentry,buffer)))
 			goto out;
 	}
@@ -172,7 +179,8 @@ out:
  */
 
 static int 
-fill_write_buffer(struct sysfs_buffer * buffer, const char __user *
buf, size_t count)
+fill_write_buffer(struct sysfs_buffer *buffer, const char __user * buf,
+		  size_t count, size_t pos)
 {
 	int error;
 
@@ -181,10 +189,11 @@ fill_write_buffer(struct sysfs_buffer * 
 	if (!buffer->page)
 		return -ENOMEM;
 
-	if (count >= PAGE_SIZE)
-		count = PAGE_SIZE - 1;
+	if (count + pos > PAGE_SIZE)
+		count = (PAGE_SIZE - 1) - pos;
 	error = copy_from_user(buffer->page,buf,count);
-	buffer->needs_read_fill = 1;
+	buffer->needs_write_flush = 1;
+        buffer->count = pos + count;
 	return error ? -EFAULT : count;
 }
 
@@ -200,13 +209,13 @@ fill_write_buffer(struct sysfs_buffer * 
  */
 
 static int 
-flush_write_buffer(struct dentry * dentry, struct sysfs_buffer *
buffer, size_t count)
+flush_write_buffer(struct dentry * dentry, struct sysfs_buffer *
buffer)
 {
 	struct attribute * attr = to_attr(dentry);
 	struct kobject * kobj = to_kobj(dentry->d_parent);
 	struct sysfs_ops * ops = buffer->ops;
 
-	return ops->store(kobj,attr,buffer->page,count);
+	return ops->store(kobj,attr, buffer->page, buffer->count);
 }
 
 
@@ -219,12 +228,9 @@ flush_write_buffer(struct dentry * dentr
  *
  *	Similar to sysfs_read_file(), though working in the opposite
direction.
  *	We allocate and fill the data from the user in
fill_write_buffer(),
- *	then push it to the kobject in flush_write_buffer().
- *	There is no easy way for us to know if userspace is only doing a
partial
- *	write, so we don't support them. We expect the entire buffer to
come
- *	on the first write. 
- *	Hint: if you're writing a value, first read the file, modify
only the
- *	the value you're changing, then write entire buffer back. 
+ *	but don't push it to the kobject until the file is closed.
+ *      This allows for buffered writes, but unfortunately also hides
error
+ *      codes returned individual store functions until close time.
  */
 
 static ssize_t
@@ -232,10 +238,10 @@ sysfs_write_file(struct file *file, cons
 {
 	struct sysfs_buffer * buffer = file->private_data;
 
+	if (*ppos >= PAGE_SIZE)
+		return -ENOSPC;
 	down(&buffer->sem);
-	count = fill_write_buffer(buffer,buf,count);
-	if (count > 0)
-		count = flush_write_buffer(file->f_dentry,buffer,count);
+	count = fill_write_buffer(buffer, buf, count, *ppos);
 	if (count > 0)
 		*ppos += count;
 	up(&buffer->sem);
@@ -275,6 +281,11 @@ static int check_perm(struct inode * ino
 	if (!ops)
 		goto Eaccess;
 
+	/* Is the file is open for append?  Sorry, we don't do that. */
+	if (file->f_flags & O_APPEND) {
+		goto Einval;
+	}
+
 	/* File needs write support.
 	 * The inode's perms must say it's ok, 
 	 * and we must have a store method.
@@ -302,11 +313,14 @@ static int check_perm(struct inode * ino
 	if (buffer) {
 		memset(buffer,0,sizeof(struct sysfs_buffer));
 		init_MUTEX(&buffer->sem);
-		buffer->needs_read_fill = 1;
 		buffer->ops = ops;
 		file->private_data = buffer;
 	} else
 		error = -ENOMEM;
+
+	/*  Set mode bits to disallow seeking.  */
+	file->f_mode &= ~(FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE);
+
 	goto Done;
 
  Einval:
@@ -332,6 +346,17 @@ static int sysfs_release(struct inode * 
 	struct attribute * attr = to_attr(filp->f_dentry);
 	struct module * owner = attr->owner;
 	struct sysfs_buffer * buffer = filp->private_data;
+        int ret;
+
+	/* If data has been written to the file, then flush it
+	 * back to the kobject's store function here.
+	 */
+	if (buffer && kobj) {
+		down(&buffer->sem);
+		if (buffer->needs_write_flush)
+			ret = flush_write_buffer(filp->f_dentry,
buffer);
+		up(&buffer->sem);
+	}
 
 	if (kobj) 
 		kobject_put(kobj);
@@ -343,13 +368,16 @@ static int sysfs_release(struct inode * 
 			free_page((unsigned long)buffer->page);
 		kfree(buffer);
 	}
-	return 0;
+	/* If flush_write_buffer returned an error, pass it up.
+	 * Otherwise, return success.
+	 */
+	return (ret < 0 ? ret : 0);
 }
 
 struct file_operations sysfs_file_operations = {
 	.read		= sysfs_read_file,
 	.write		= sysfs_write_file,
-	.llseek		= generic_file_llseek,
+	.llseek		= no_llseek,
 	.open		= sysfs_open_file,
 	.release	= sysfs_release,
 };

Re: [PATCH] sysfs write fixes

[Greg KH]

On Fri, Jan 21, 2005 at 01:44:45PM -0800, Williams, Mitch A wrote:
> This patch corrects some errors that we saw while writing to sysfs
> files.
>  - Attempts to open the file in append mode result in error
>  - Writes are buffered and flushed to the kobject owner during close
>  - Attempts to seek on sysfs files result in error
> 
> Generated from 2.6.11-rc1.
> 
> It's not a big patch, but if you'd like it whacked up into smaller ones,
> I'll
> be glad to do so.

Please split it up into different patches in different emails.  Also,
fix your email client, the patch was line wrapped.

And don't add this:

> @@ -1,5 +1,11 @@
>  /*
>   * file.c - operations for regular (text) files.
> + *
> + * Changes:
> + * 2004/01/21 Mitch Williams <mitch.a.williams at intel.com>
> + *   - Changed llseek method to be no_llseek.
> + *   - Opening a file for append now returns an error.
> + *   - Writes are now buffered and flushed at close
>   */

Changes show up in the change log comments, not within the files.
Otherwise, over time they grow out of control.

thanks,

greg k-h