[PATCH] cpusets: alternative fix for possible race in cpuset_tasks_read()

From: Paul Jackson (pj_at_sgi.com)
Date: 09/11/04

  • Next message: Paul Jackson: "Re: [rfc][patch] 1/2 Additional cpuset features" 
    Date:	Sat, 11 Sep 2004 01:01:20 -0700
To: Simon Derr <simon.derr@bull.net>

    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/
  • Next message: Paul Jackson: "Re: [rfc][patch] 1/2 Additional cpuset features"

    Relevant Pages