Messages in this thread Patch in this message | ![/](/images/icornerl.gif) | | Date | Sat, 11 Sep 2004 01:01:20 -0700 | From | Paul Jackson <> | Subject | [PATCH] cpusets: alternative fix for possible race in cpuset_tasks_read() |
| |
Here's an alternative fix for the race condition on read that Simon reports.
Andrew,
Don't apply this one yet, until Simon Derr gets a chance to compare with his alternative patch, and render his analysis.
Move the code that sets up the character buffer of text to read out when reading a "tasks" file from the read routine to the open routine.
Multiple cloned threads could be doing the first read on a shared file descriptor open on a "tasks" file, resulting in confused or leaked kernel memory as multiple threads initialized the same file private_data at the same time. Rather than add locks to the initialization code, move it into the open(), where it belongs anyway.
Signed-off-by: Paul Jackson
Index: 2.6.9-rc1-mm4/kernel/cpuset.c =================================================================== --- 2.6.9-rc1-mm4.orig/kernel/cpuset.c 2004-09-10 15:27:32.000000000 -0700 +++ 2.6.9-rc1-mm4/kernel/cpuset.c 2004-09-10 21:20:02.000000000 -0700 @@ -1034,7 +1034,7 @@ static int pid_array_to_buf(char *buf, i return cnt; } -static inline struct ctr_struct *cpuset_tasks_mkctr(struct file *file) +static int cpuset_tasks_open(struct inode *unused, struct file *file) { struct cpuset *cs = __d_cs(file->f_dentry->d_parent); struct ctr_struct *ctr; @@ -1069,14 +1069,14 @@ static inline struct ctr_struct *cpuset_ kfree(pidarray); file->private_data = (void *)ctr; - return ctr; + return 0; err2: kfree(pidarray); err1: kfree(ctr); err0: - return NULL; + return -ENOMEM; } static ssize_t cpuset_tasks_read(struct file *file, char __user *buf, @@ -1084,13 +1084,6 @@ static ssize_t cpuset_tasks_read(struct { struct ctr_struct *ctr = (struct ctr_struct *)file->private_data; - /* allocate buffer and fill it on first call to read() */ - if (!ctr) { - ctr = cpuset_tasks_mkctr(file); - if (!ctr) - return -ENOMEM; - } - if (*ppos + nbytes > ctr->bufsz) nbytes = ctr->bufsz - *ppos; if (copy_to_user(buf, ctr->buf + *ppos, nbytes)) @@ -1121,6 +1114,7 @@ static int cpuset_tasks_release(struct i static struct cftype cft_tasks = { .name = "tasks", + .open = cpuset_tasks_open, .read = cpuset_tasks_read, .release = cpuset_tasks_release, .private = FILE_TASKLIST,
-- I won't rest till it's the best ... Programmer, Linux Scalability Paul Jackson <pj@sgi.com> 1.650.933.1373 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| ![\](/images/icornerr.gif) |