Commit 185b8e24 authored by Patrick Mochel's avatar Patrick Mochel

sysfs: Rewrite binary file handling.

From Matthew Wilcox.

 - 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.
parent 65f99db2
...@@ -2,170 +2,118 @@ ...@@ -2,170 +2,118 @@
* bin.c - binary file operations for sysfs. * bin.c - binary file operations for sysfs.
*/ */
#include <linux/errno.h>
#include <linux/fs.h> #include <linux/fs.h>
#include <linux/module.h>
#include <linux/kobject.h> #include <linux/kobject.h>
#include <linux/module.h>
#include <linux/slab.h>
#include <asm/uaccess.h> #include <asm/uaccess.h>
#include "sysfs.h" #include "sysfs.h"
static struct file_operations bin_fops; static int
fill_read(struct dentry *dentry, char *buffer, loff_t off, size_t count)
static int fill_read(struct file * file, struct sysfs_bin_buffer * buffer)
{ {
struct bin_attribute * attr = file->f_dentry->d_fsdata; struct bin_attribute * attr = dentry->d_fsdata;
struct kobject * kobj = file->f_dentry->d_parent->d_fsdata; struct kobject * kobj = dentry->d_parent->d_fsdata;
if (!buffer->data) return attr->read(kobj, buffer, off, count);
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;
} }
static ssize_t static ssize_t
read(struct file * file, char * userbuf, size_t count, loff_t * off) 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; int ret;
ret = fill_read(file,buffer); if (offs > size)
if (ret) return 0;
goto Done; if (offs + count > size)
count = size - offs;
buffer->offset = *off;
if (count > (buffer->size - *off)) ret = fill_read(dentry, buffer, offs, count);
count = buffer->size - *off; if (ret < 0)
goto Done;
count = ret;
buffer->count = count; ret = -EFAULT;
if (copy_to_user(userbuf, buffer + offs, count) != 0)
goto Done;
ret = flush_read(file,userbuf,buffer); *off = offs + count;
if (!ret) {
*off += count;
ret = count; ret = count;
}
Done: Done:
if (buffer && buffer->data) {
kfree(buffer->data);
buffer->data = NULL;
}
return ret; return ret;
} }
int alloc_buf_data(struct sysfs_bin_buffer * buffer) static int
flush_write(struct dentry *dentry, char *buffer, loff_t offset, size_t count)
{ {
buffer->data = kmalloc(buffer->count,GFP_KERNEL); struct bin_attribute *attr = dentry->d_fsdata;
if (buffer->data) { struct kobject *kobj = dentry->d_parent->d_fsdata;
memset(buffer->data,0,buffer->count);
return 0;
} else
return -ENOMEM;
}
static int fill_write(struct file * file, const char * userbuf, return attr->write(kobj, buffer, offset, count);
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)
{
struct bin_attribute * attr = file->f_dentry->d_fsdata;
struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
return attr->write(kobj,buffer);
} }
static ssize_t write(struct file * file, const char * userbuf, static ssize_t write(struct file * file, const char * userbuf,
size_t count, loff_t * off) 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; int ret;
if (count > PAGE_SIZE) if (offs > size)
count = PAGE_SIZE; return 0;
buffer->count = count; if (offs + count > size)
count = size - offs;
ret = alloc_buf_data(buffer);
if (ret)
goto Done;
ret = fill_write(file,userbuf,buffer); ret = -EFAULT;
if (ret) if (copy_from_user(buffer + offs, userbuf, count))
goto Done; goto Done;
ret = flush_write(file,userbuf,buffer); count = flush_write(dentry, buffer, offs, count);
if (ret > 0) if (count > 0)
*off += count; *off = offs + count;
ret = 0;
Done: Done:
if (buffer->data) {
kfree(buffer->data);
buffer->data = NULL;
}
return ret; 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 kobject * kobj = kobject_get(file->f_dentry->d_parent->d_fsdata);
struct bin_attribute * attr = file->f_dentry->d_fsdata; struct bin_attribute * attr = file->f_dentry->d_fsdata;
struct sysfs_bin_buffer * buffer; int error = -EINVAL;
int error = 0;
if (!kobj || !attr) if (!kobj || !attr)
goto Einval; goto Done;
/* 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. error = -EACCES;
* The inode's perms must say it's ok, and we there if ((file->f_mode & FMODE_WRITE) && !attr->write)
* must be a show method for it. goto Done;
*/ if ((file->f_mode & FMODE_READ) && !attr->read)
if (file->f_mode & FMODE_READ) { goto Done;
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; error = -ENOMEM;
file->private_data = kmalloc(attr->size, GFP_KERNEL);
if (!file->private_data)
goto Done; goto Done;
Einval: error = 0;
error = -EINVAL;
goto Done;
Eaccess:
error = -EACCES;
Done: Done:
if (error && kobj) if (error && kobj)
kobject_put(kobj); kobject_put(kobj);
return error; 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) static int release(struct inode * inode, struct file * file)
{ {
struct kobject * kobj = file->f_dentry->d_parent->d_fsdata; struct kobject * kobj = file->f_dentry->d_parent->d_fsdata;
...@@ -173,7 +121,6 @@ static int release(struct inode * inode, struct file * file) ...@@ -173,7 +121,6 @@ static int release(struct inode * inode, struct file * file)
if (kobj) if (kobj)
kobject_put(kobj); kobject_put(kobj);
if (buffer)
kfree(buffer); kfree(buffer);
return 0; return 0;
} }
......
...@@ -16,18 +16,11 @@ struct attribute { ...@@ -16,18 +16,11 @@ struct attribute {
mode_t mode; mode_t mode;
}; };
struct sysfs_bin_buffer {
u8 * data;
size_t size;
size_t count;
loff_t offset;
};
struct bin_attribute { struct bin_attribute {
struct attribute attr; struct attribute attr;
size_t size; size_t size;
ssize_t (*read)(struct kobject *, struct sysfs_bin_buffer *); ssize_t (*read)(struct kobject *, char *, loff_t, size_t);
ssize_t (*write)(struct kobject *, struct sysfs_bin_buffer *); ssize_t (*write)(struct kobject *, char *, loff_t, size_t);
}; };
struct sysfs_ops { struct sysfs_ops {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment