CVE-2009-3547: Linux kernel Pipe NULL Pointer Dereference Race Condition

This awesome vulnerability was discovered by Earl Chew and quickly noticed and classified as a vulnerability by Eugene Teo of Red Hat.
This vulnerability is extremely straightforward to exploit after all the exploit releases of spender on NULL pointer dereference bugs. So, here is some code from fs/pipe.c as seen in 2.6.29 release of the Linux kernel…

const struct file_operations write_pipefifo_fops = {
   ...
        .open           = pipe_write_open,
   ...
};
   ...
static int
pipe_write_open(struct inode *inode, struct file *filp)
{
        mutex_lock(&inode->i_mutex);
        inode->i_pipe->writers++;
        mutex_unlock(&inode->i_mutex);

        return 0;
}

The code isn’t that hard to understand. When you open a pipe for writing, this routine is called and it will increment the ‘inode->i_pipe->writers’ that represents the current number of writers to that pipe as you might have guessed. Of course, it is doing this in a MUTEX lock to avoid race conditions with other code that might change that counter’s value.
The equivalent functions that release a pipe are the following three:

static int
pipe_read_release(struct inode *inode, struct file *filp)
{
        return pipe_release(inode, 1, 0);
}

static int
pipe_write_release(struct inode *inode, struct file *filp)
{
        return pipe_release(inode, 0, 1);
}

static int
pipe_rdwr_release(struct inode *inode, struct file *filp)
{
        int decr, decw;

        decr = (filp->f_mode & FMODE_READ) != 0;
        decw = (filp->f_mode & FMODE_WRITE) != 0;
        return pipe_release(inode, decr, decw);
}

If a process attempts to open a pipe while another one releases it, and assuming that the release happens first, the ‘open’ routines will lead to a NULL pointer dereference since ‘inode->i_pipe’ would be NULL.
This amazing vulnerability was fixed by applying the following patch:

 pipe_read_open(struct inode *inode, struct file *filp)
 {
-       /* We could have perhaps used atomic_t, but this and friends
-          below are the only places.  So it doesn't seem worthwhile.  */
+       int ret = -ENOENT;
+
        mutex_lock(&inode->i_mutex);
-       inode->i_pipe->readers++;
+
+       if (inode->i_pipe) {
+               ret = 0;
+               inode->i_pipe->readers++;
+       }
+
        mutex_unlock(&inode->i_mutex);
 
-       return 0;
+       return ret;
 }
 
 static int
 pipe_write_open(struct inode *inode, struct file *filp)
 {
+       int ret = -ENOENT;
+
        mutex_lock(&inode->i_mutex);
-       inode->i_pipe->writers++;
+
+       if (inode->i_pipe) {
+               ret = 0;
+               inode->i_pipe->writers++;
+       }
+
        mutex_unlock(&inode->i_mutex);
 
-       return 0;
+       return ret;
 }
 
 static int
 pipe_rdwr_open(struct inode *inode, struct file *filp)
 {
+       int ret = -ENOENT;
+
        mutex_lock(&inode->i_mutex);
-       if (filp->f_mode & FMODE_READ)
-               inode->i_pipe->readers++;
-       if (filp->f_mode & FMODE_WRITE)
-               inode->i_pipe->writers++;
+
+       if (inode->i_pipe) {
+               ret = 0;
+               if (filp->f_mode & FMODE_READ)
+                       inode->i_pipe->readers++;
+               if (filp->f_mode & FMODE_WRITE)
+                       inode->i_pipe->writers++;
+       }
+
        mutex_unlock(&inode->i_mutex);
 
-       return 0;
+       return ret;
 }

As you can see, it is a simple series of NULL checks against ‘inode->i_pipe’ on pipe_read_open(), pipe_write_open() and pipe_rdwr_open().
This results in a completely user control increment of an arbitrary value located at ‘NULL->readers’ of ‘NULL->writers’. Does this reminds you anything? :P
There might also be easier ways to exploit this. Anyway, too bad that such a nice bug was killed. :(