sysfs bin file fixes
PATCH 1/3 sysfs bin file fixes
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.
fs/sysfs/bin.c +60 -113
@@ -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 ; }
include/linux/sysfs.h +2 -9
@@ -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 {
Signature · 3 lines
"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
Quoted 2 lines · Matthew Wilcox
Completely rewrite sysfs' handling of binary files:
Thanks a lot for doing this. I'll apply this, and push it on.
-pat