Skip to content

Commit

Permalink
debugfs: fix race in u32_array_read and allocate array at open
Browse files Browse the repository at this point in the history
u32_array_open() is racy when multiple threads read from a file with a
seek position of zero, i.e. when two or more simultaneous reads are
occurring after the non-seekable files are created.  It is possible that
file->private_data is double-freed because the threads races between

	kfree(file->private-data);

and

	file->private_data = NULL;

The fix is to only do format_array_alloc() when the file is opened and
free it when it is closed.

Note that because the file has always been non-seekable, you can't open
it and read it multiple times anyway, so the data has always been
generated just once.  The difference is that now it is generated at open
time rather than at the time of the first read, and that avoids the
race.

Reported-by: Dave Jones <[email protected]>
Acked-by: Konrad Rzeszutek Wilk <[email protected]>
Tested-by: Raghavendra <[email protected]>
Signed-off-by: David Rientjes <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
rientjes authored and torvalds committed Sep 21, 2012
1 parent c46de22 commit 3604885
Showing 1 changed file with 11 additions and 22 deletions.
33 changes: 11 additions & 22 deletions fs/debugfs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -526,12 +526,6 @@ struct array_data {
u32 elements;
};

static int u32_array_open(struct inode *inode, struct file *file)
{
file->private_data = NULL;
return nonseekable_open(inode, file);
}

static size_t format_array(char *buf, size_t bufsize, const char *fmt,
u32 *array, u32 array_size)
{
Expand Down Expand Up @@ -573,26 +567,21 @@ static char *format_array_alloc(const char *fmt, u32 *array,
return ret;
}

static ssize_t u32_array_read(struct file *file, char __user *buf, size_t len,
loff_t *ppos)
static int u32_array_open(struct inode *inode, struct file *file)
{
struct inode *inode = file->f_path.dentry->d_inode;
struct array_data *data = inode->i_private;
size_t size;

if (*ppos == 0) {
if (file->private_data) {
kfree(file->private_data);
file->private_data = NULL;
}

file->private_data = format_array_alloc("%u", data->array,
data->elements);
}
file->private_data = format_array_alloc("%u", data->array,
data->elements);
if (!file->private_data)
return -ENOMEM;
return nonseekable_open(inode, file);
}

size = 0;
if (file->private_data)
size = strlen(file->private_data);
static ssize_t u32_array_read(struct file *file, char __user *buf, size_t len,
loff_t *ppos)
{
size_t size = strlen(file->private_data);

return simple_read_from_buffer(buf, len, ppos,
file->private_data, size);
Expand Down

0 comments on commit 3604885

Please sign in to comment.