From: Jeff Moyer <jmoyer@redhat.com>
Date: Mon, 26 Oct 2009 17:00:49 -0400
Subject: [fs] pipe.c null pointer dereference
Message-id: x4963a1vpj2.fsf@segfault.boston.devel.redhat.com
O-Subject: [kernel team] [rhel 5 patch, CVE-2009-3547] fs: pipe.c null pointer dereference
Bugzilla: 530939
RH-Acked-by: Jeff Layton <jlayton@redhat.com>
RH-Acked-by: Danny Feng <dfeng@redhat.com>
RH-Acked-by: David Howells <dhowells@redhat.com>
RH-Acked-by: Cong Wang <amwang@redhat.com>
CVE: CVE-2009-3547

Hi,

This bug was reported upstream and fixed by the following commit:
  ad3960243e55320d74195fb85c975e0a8cc4466c

Americo Wang summed up the problem thusly:

  "So, if I understand you correctly, you mean we have a small window
   between calling sys_open() and fifo_open(), during this little period,
   we don't have i_mutex held, thun another process have a chance to
   release that pipe and make i_pipe NULL. Right?" [1]

There is a reproducer in the upstream posting which the reporter used to
verify that the patch solved the issue.  The upstream patch applied
directly to RHEL 5.  I have built the kernel, but have not yet even
booted it.  I was asked to post this for review to get into a kernel
respin tomorrow.

I plan to test this tomorrow.  For now, comments would be greatly
appreciated.

Cheers,
Jeff

[1] http://lkml.org/lkml/2009/10/21/42

diff --git a/fs/pipe.c b/fs/pipe.c
index 717a9bb..5116022 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -697,36 +697,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;
 }
 
 /*