[PATCH] fs: pipe.c null pointer dereference - CVE-2009-3547

[PATCH] fs: pipe.c null pointer dereference - CVE-2009-3547

[Shaun Reitan]

We've been applying this patch since the fix was discovered but i just 
realized yesterday when building a new kernel that the Xen kernel does 
not have this fix applied yet.

I also have verified that this exploit works to gain root access on the 
current http://xenbits.xensource.com/linux-2.6.18-xen.hg branch

-- 
Shaun Retian
Chief Technical Officer
Network Data Center Host, Inc.
http://www.ndchost.com

-- PATCH --

 From ad3960243e55320d74195fb85c975e0a8cc4466c Mon Sep 17 00:00:00 2001
From: Earl Chew <earl_chew@agilent.com>
Date: Mon, 19 Oct 2009 15:55:41 -0700
Subject: [PATCH] fs: pipe.c null pointer dereference

This patch fixes a null pointer exception in pipe_rdwr_open() which
generates the stack trace:

 > Unable to handle kernel NULL pointer dereference at 0000000000000028 RIP:
 >  [<ffffffff802899a5>] pipe_rdwr_open+0x35/0x70
 >  [<ffffffff8028125c>] __dentry_open+0x13c/0x230
 >  [<ffffffff8028143d>] do_filp_open+0x2d/0x40
 >  [<ffffffff802814aa>] do_sys_open+0x5a/0x100
 >  [<ffffffff8021faf3>] sysenter_do_call+0x1b/0x67

The failure mode is triggered by an attempt to open an anonymous
pipe via /proc/pid/fd/* as exemplified by this script:

=============================================================
while : ; do
    { echo y ; sleep 1 ; } | { while read ; do echo z$REPLY; done ; } &
    PID=$!
    OUT=$(ps -efl | grep 'sleep 1' | grep -v grep |
         { read PID REST ; echo $PID; } )
    OUT="${OUT%% *}"
    DELAY=$((RANDOM * 1000 / 32768))
    usleep $((DELAY * 1000 + RANDOM % 1000 ))
    echo n > /proc/$OUT/fd/1                 # Trigger defect
done
=============================================================

Note that the failure window is quite small and I could only
reliably reproduce the defect by inserting a small delay
in pipe_rdwr_open(). For example:

  static int
  pipe_rdwr_open(struct inode *inode, struct file *filp)
  {
        msleep(100);
        mutex_lock(&inode->i_mutex);

Although the defect was observed in pipe_rdwr_open(), I think it
makes sense to replicate the change through all the pipe_*_open()
functions.

The core of the change is to verify that inode->i_pipe has not
been released before attempting to manipulate it. If inode->i_pipe
is no longer present, return ENOENT to indicate so.

The comment about potentially using atomic_t for i_pipe->readers
and i_pipe->writers has also been removed because it is no longer
relevant in this context. The inode->i_mutex lock must be used so
that inode->i_pipe can be dealt with correctly.

Signed-off-by: Earl Chew <earl_chew@agilent.com>
Cc: stable@kernel.org
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
  fs/pipe.c |   41 ++++++++++++++++++++++++++++++-----------
  1 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 52c4151..ae17d02 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -777,36 +777,55 @@ pipe_rdwr_release(struct inode *inode, struct file 
*filp)
  static int
  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;
  }

  /*
--
1.7.3.2

Re: [PATCH] fs: pipe.c null pointer dereference - CVE-2009-3547

[Keir Fraser]

On 22/11/2010 16:27, "Shaun Reitan" <mailinglists@unix-scripts.com> wrote:

> We've been applying this patch since the fix was discovered but i just
> realized yesterday when building a new kernel that the Xen kernel does
> not have this fix applied yet.
> 
> I also have verified that this exploit works to gain root access on the
> current http://xenbits.xensource.com/linux-2.6.18-xen.hg branch

It has to be said, very clearly, that our 2.6.18 tree is only really of use
now as a repository of Xen patches for vendors to pull into their own,
*properly maintained and secured* kernels. We are very interested in fixing
Xen-related security issues in our 2.6.18 tree (precisely because others use
it as a repository of good Xen patches). We are less interested in general
kernel fixes, although of course as a matter of good form we will consider a
security fix such as you propose. However, the patch you supplied does not
apply to the 2.6.18 tree.

 Thanks,
 Keir

Re: [PATCH] fs: pipe.c null pointer dereference - CVE-2009-3547

[Shaun Reitan]

On 11/22/2010 11:24 AM, Keir Fraser wrote:
> On 22/11/2010 16:27, "Shaun Reitan"<mailinglists@unix-scripts.com>  wrote:
>
>> We've been applying this patch since the fix was discovered but i just
>> realized yesterday when building a new kernel that the Xen kernel does
>> not have this fix applied yet.
>>
>> I also have verified that this exploit works to gain root access on the
>> current http://xenbits.xensource.com/linux-2.6.18-xen.hg branch
>
> It has to be said, very clearly, that our 2.6.18 tree is only really of use
> now as a repository of Xen patches for vendors to pull into their own,
> *properly maintained and secured* kernels. We are very interested in fixing
> Xen-related security issues in our 2.6.18 tree (precisely because others use
> it as a repository of good Xen patches). We are less interested in general
> kernel fixes, although of course as a matter of good form we will consider a
> security fix such as you propose. However, the patch you supplied does not
> apply to the 2.6.18 tree.
>
>   Thanks,
>   Keir

I see, good to know, thanks!

-- 
Shaun Retian
Chief Technical Officer
Network Data Center Host, Inc.
http://www.ndchost.com