Skip to content

Commit

Permalink
fs: pipe.c null pointer dereference
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Earl Chew authored and Linus Torvalds committed Oct 21, 2009
1 parent 2fdc246 commit ad39602
Showing 1 changed file with 30 additions and 11 deletions.
41 changes: 30 additions & 11 deletions fs/pipe.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/*
Expand Down

0 comments on commit ad39602

Please sign in to comment.