On Tue, 18 Sep 2012, Konrad Rzeszutek Wilk wrote:
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 2340f69..309b235 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -524,6 +524,7 @@ EXPORT_SYMBOL_GPL(debugfs_create_blob);
> struct array_data {
> void *array;
> u32 elements;
> + struct mutex lock;
This should be a spinlock.
> };
>
> static int u32_array_open(struct inode *inode, struct file *file)
> @@ -580,6 +581,7 @@ static ssize_t u32_array_read(struct file *file, char
> __user *buf, size_t len,
> struct array_data *data = inode->i_private;
> size_t size;
>
> + mutex_lock(&data->lock);
> if (*ppos == 0) {
> if (file->private_data) {
> kfree(file->private_data);
> @@ -594,6 +596,7 @@ static ssize_t u32_array_read(struct file *file, char
> __user *buf, size_t len,
> if (file->private_data)
> size = strlen(file->private_data);
>
> + mutex_unlock(&data->lock);
> return simple_read_from_buffer(buf, len, ppos,
> file->private_data, size);
> }
Your critical section isn't entirely covered since you're still accessing
file->private_data in the call to simple_read_from_buffer(). What happens
if a concurrent reader does file->private_data = NULL immediately after
your unlock?
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
