Completely rewrite sysfs' handling of binary files:
- Remove sysfs_bin_buffer. It's a security hole.
- Remove checks for permissions; the VFS does that.
- Validate offset & count at the top level.
- Allow lower levels to return less data than was asked for.
- Allocate buffer at open & free it at close, not on each read/write.
diff -urpNX dontdiff linux-2.5.66/fs/sysfs/bin.c linux-2.5.66-laptop/fs/sysfs/bin.c
--- linux-2.5.66/fs/sysfs/bin.c 2003-04-07 09:59:08.000000000 -0500
+++ linux-2.5.66-laptop/fs/sysfs/bin.c 2003-04-07 11:01:19.000000000 -0500
@@ -2,168 +2,116 @@
* bin.c - binary file operations for sysfs.
*/
+#include <linux/errno.h>
#include <linux/fs.h>
-#include <linux/module.h>
#include <linux/kobject.h>
+#include <linux/module.h>
+#include <linux/slab.h>
#include "sysfs.h"
-static struct file_operations bin_fops;
-
-static int fill_read(struct file * file, struct sysfs_bin_buffer * buffer)
+static int
+fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)
{
- struct bin_attribute * attr = file->f_dentry->d_fsdata;
- struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+ struct bin_attribute * attr = dentry->d_fsdata;
+ struct kobject * kobj = dentry->d_parent->d_fsdata;
- if (!buffer->data)
- attr->read(kobj,buffer);
- return buffer->size ? 0 : -ENOENT;
-}
-
-static int flush_read(struct file * file, char * userbuf,
- struct sysfs_bin_buffer * buffer)
-{
- return copy_to_user(userbuf,buffer->data + buffer->offset,buffer->count) ?
- -EFAULT : 0;
+ return attr->read(kobj, buffer, off, count);
}
static ssize_t
read(struct file * file, char * userbuf, size_t count, loff_t * off)
{
- struct sysfs_bin_buffer * buffer = file->private_data;
+ char *buffer = file->private_data;
+ struct dentry *dentry = file->f_dentry;
+ int size = dentry->d_inode->i_size;
+ loff_t offs = *off;
int ret;
- ret = fill_read(file,buffer);
- if (ret)
- goto Done;
+ if (offs > size)
+ return 0;
+ if (offs + count > size)
+ count = size - offs;
- buffer->offset = *off;
+ ret = fill_read(dentry, buffer, offs, count);
+ if (ret < 0)
+ goto Done;
+ count = ret;
- if (count > (buffer->size - *off))
- count = buffer->size - *off;
+ ret = -EFAULT;
+ if (copy_to_user(userbuf, buffer + offs, count) != 0)
+ goto Done;
- buffer->count = count;
+ *off = offs + count;
+ ret = count;
- ret = flush_read(file,userbuf,buffer);
- if (!ret) {
- *off += count;
- ret = count;
- }
Done:
- if (buffer && buffer->data) {
- kfree(buffer->data);
- buffer->data = NULL;
- }
return ret;
}
-int alloc_buf_data(struct sysfs_bin_buffer * buffer)
-{
- buffer->data = kmalloc(buffer->count,GFP_KERNEL);
- if (buffer->data) {
- memset(buffer->data,0,buffer->count);
- return 0;
- } else
- return -ENOMEM;
-}
-
-static int fill_write(struct file * file, const char * userbuf,
- struct sysfs_bin_buffer * buffer)
-{
- return copy_from_user(buffer->data,userbuf,buffer->count) ?
- -EFAULT : 0;
-}
-
-static int flush_write(struct file * file, const char * userbuf,
- struct sysfs_bin_buffer * buffer)
+static int
+flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
{
- struct bin_attribute * attr = file->f_dentry->d_fsdata;
- struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
+ struct bin_attribute *attr = dentry->d_fsdata;
+ struct kobject *kobj = dentry->d_parent->d_fsdata;
- return attr->write(kobj,buffer);
+ return attr->write(kobj, buffer, offset, count);
}
static ssize_t write(struct file * file, const char * userbuf,
size_t count, loff_t * off)
{
- struct sysfs_bin_buffer * buffer = file->private_data;
+ char *buffer = file->private_data;
+ struct dentry *dentry = file->f_dentry;
+ int size = dentry->d_inode->i_size;
+ loff_t offs = *off;
int ret;
- if (count > PAGE_SIZE)
- count = PAGE_SIZE;
- buffer->count = count;
-
- ret = alloc_buf_data(buffer);
- if (ret)
- goto Done;
+ if (offs > size)
+ return 0;
+ if (offs + count > size)
+ count = size - offs;
- ret = fill_write(file,userbuf,buffer);
- if (ret)
+ ret = -EFAULT;
+ if (copy_from_user(buffer + offs, userbuf, count))
goto Done;
- ret = flush_write(file,userbuf,buffer);
- if (ret > 0)
- *off += count;
+ count = flush_write(dentry, buffer, offs, count);
+ if (count > 0)
+ *off = offs + count;
+ ret = 0;
Done:
- if (buffer->data) {
- kfree(buffer->data);
- buffer->data = NULL;
- }
return ret;
}
-static int check_perm(struct inode * inode, struct file * file)
+static int open(struct inode * inode, struct file * file)
{
struct kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
struct bin_attribute * attr = file->f_dentry->d_fsdata;
- struct sysfs_bin_buffer * buffer;
- int error = 0;
+ int error = -EINVAL;
if (!kobj || !attr)
- goto Einval;
-
- /* File needs write support.
- * The inode's perms must say it's ok,
- * and we must have a store method.
- */
- if (file->f_mode & FMODE_WRITE) {
- if (!(inode->i_mode & S_IWUGO) || !attr->write)
- goto Eaccess;
- }
-
- /* File needs read support.
- * The inode's perms must say it's ok, and we there
- * must be a show method for it.
- */
- if (file->f_mode & FMODE_READ) {
- if (!(inode->i_mode & S_IRUGO) || !attr->read)
- goto Eaccess;
- }
-
- buffer = kmalloc(sizeof(struct sysfs_bin_buffer),GFP_KERNEL);
- if (buffer) {
- memset(buffer,0,sizeof(struct sysfs_bin_buffer));
- file->private_data = buffer;
- } else
- error = -ENOMEM;
- goto Done;
+ goto Done;
- Einval:
- error = -EINVAL;
- goto Done;
- Eaccess:
error = -EACCES;
+ if ((file->f_mode & FMODE_WRITE) && !attr->write)
+ goto Done;
+ if ((file->f_mode & FMODE_READ) && !attr->read)
+ goto Done;
+
+ error = -ENOMEM;
+ file->private_data = kmalloc(attr->size, GFP_KERNEL);
+ if (!file->private_data)
+ goto Done;
+
+ error = 0;
+
Done:
if (error && kobj)
kobject_put(kobj);
return error;
}
-static int open(struct inode * inode, struct file * file)
-{
- return check_perm(inode,file);
-}
-
static int release(struct inode * inode, struct file * file)
{
struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
@@ -171,8 +119,7 @@ static int release(struct inode * inode,
if (kobj)
kobject_put(kobj);
- if (buffer)
- kfree(buffer);
+ kfree(buffer);
return 0;
}
diff -urpNX dontdiff linux-2.5.66/include/linux/sysfs.h linux-2.5.66-laptop/include/linux/sysfs.h
--- linux-2.5.66/include/linux/sysfs.h 2003-04-04 08:42:19.000000000 -0600
+++ linux-2.5.66-laptop/include/linux/sysfs.h 2003-04-07 09:54:50.000000000 -0500
@@ -16,18 +16,11 @@ struct attribute {
mode_t mode;
};
-struct sysfs_bin_buffer {
- u8 * data;
- size_t size;
- size_t count;
- loff_t offset;
-};
-
struct bin_attribute {
struct attribute attr;
size_t size;
- ssize_t (*read)(struct kobject *, struct sysfs_bin_buffer *);
- ssize_t (*write)(struct kobject *, struct sysfs_bin_buffer *);
+ ssize_t (*read)(struct kobject *, char *, loff_t, size_t);
+ ssize_t (*write)(struct kobject *, char *, loff_t, size_t);
};
struct sysfs_ops {
-- "It's not Hollywood. War is real, war is primarily not about defeat or victory, it is about death. I've seen thousands and thousands of dead bodies. Do you think I want to have an academic debate on this subject?" -- Robert Fisk - 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/
This archive was generated by hypermail 2b29 : Mon Apr 07 2003 - 22:00:33 EST