On 09/19/2012 01:54 AM, David Rientjes wrote:
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.
I remember we used debugfs because traceprintks used spinlock. The code was being accessed from paravirt spinlock. Sorry for joining late (Time Zone difference) CCing Jeremy
}; 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 [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
