From 741b6cf8a565f75b3868bea8a0c4c0e247e23d24 Mon Sep 17 00:00:00 2001
From: Konstantin Belousov <kib@FreeBSD.org>
Date: Fri, 23 May 2008 11:14:03 +0000
Subject: [PATCH] Another problem caused by the knlist_cleardel() potentially
 dropping PIPE_MTX().

Since the pipe_present is cleared before (potentially) sleeping, the
second thread may enter the pipeclose() for the reciprocal pipe end.
The test at the end of the pipeclose() for the pipe_present == 0 would
succeed, allowing the second thread to free the pipe memory. First
threads then accesses the freed memory after being woken up.

Properly track the closing state of the pipe in the pipe_present.
Introduce the intermediate state that marks the pipe as mostly
dismantled but might be sleeping waiting for the knote list to be
cleared. Free the pipe pair memory only when both ends pass that point.

Debugging help and tested by:	pho
Discussed with:	jmg
MFC after:	2 weeks
---
 sys/kern/sys_pipe.c | 33 ++++++++++++++++++++++-----------
 sys/sys/pipe.h      |  7 +++++++
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index 63fc921ca06..b8c1dfeecd2 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -265,8 +265,8 @@ pipe_zone_ctor(void *mem, int size, void *arg, int flags)
 	 * one at a time.  When both are free'd, then the whole pair
 	 * is released.
 	 */
-	rpipe->pipe_present = 1;
-	wpipe->pipe_present = 1;
+	rpipe->pipe_present = PIPE_ACTIVE;
+	wpipe->pipe_present = PIPE_ACTIVE;
 
 	/*
 	 * Eventually, the MAC Framework may initialize the label
@@ -975,7 +975,8 @@ pipe_write(fp, uio, active_cred, flags, td)
 	/*
 	 * detect loss of pipe read side, issue SIGPIPE if lost.
 	 */
-	if ((!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
+	if (wpipe->pipe_present != PIPE_ACTIVE ||
+	    (wpipe->pipe_state & PIPE_EOF)) {
 		pipeunlock(wpipe);
 		PIPE_UNLOCK(rpipe);
 		return (EPIPE);
@@ -1346,13 +1347,14 @@ pipe_poll(fp, events, active_cred, td)
 			revents |= events & (POLLIN | POLLRDNORM);
 
 	if (events & (POLLOUT | POLLWRNORM))
-		if (!wpipe->pipe_present || (wpipe->pipe_state & PIPE_EOF) ||
+		if (wpipe->pipe_present != PIPE_ACTIVE ||
+		    (wpipe->pipe_state & PIPE_EOF) ||
 		    (((wpipe->pipe_state & PIPE_DIRECTW) == 0) &&
 		     (wpipe->pipe_buffer.size - wpipe->pipe_buffer.cnt) >= PIPE_BUF))
 			revents |= events & (POLLOUT | POLLWRNORM);
 
 	if ((rpipe->pipe_state & PIPE_EOF) ||
-	    (!wpipe->pipe_present) ||
+	    wpipe->pipe_present != PIPE_ACTIVE ||
 	    (wpipe->pipe_state & PIPE_EOF))
 		revents |= POLLHUP;
 
@@ -1493,7 +1495,7 @@ pipeclose(cpipe)
 	 * Disconnect from peer, if any.
 	 */
 	ppipe = cpipe->pipe_peer;
-	if (ppipe->pipe_present != 0) {
+	if (ppipe->pipe_present == PIPE_ACTIVE) {
 		pipeselwakeup(ppipe);
 
 		ppipe->pipe_state |= PIPE_EOF;
@@ -1510,16 +1512,23 @@ pipeclose(cpipe)
 	PIPE_UNLOCK(cpipe);
 	pipe_free_kmem(cpipe);
 	PIPE_LOCK(cpipe);
-	cpipe->pipe_present = 0;
+	cpipe->pipe_present = PIPE_CLOSING;
 	pipeunlock(cpipe);
+
+	/*
+	 * knlist_clear() may sleep dropping the PIPE_MTX. Set the
+	 * PIPE_FINALIZED, that allows other end to free the
+	 * pipe_pair, only after the knotes are completely dismantled.
+	 */
 	knlist_clear(&cpipe->pipe_sel.si_note, 1);
+	cpipe->pipe_present = PIPE_FINALIZED;
 	knlist_destroy(&cpipe->pipe_sel.si_note);
 
 	/*
 	 * If both endpoints are now closed, release the memory for the
 	 * pipe pair.  If not, unlock.
 	 */
-	if (ppipe->pipe_present == 0) {
+	if (ppipe->pipe_present == PIPE_FINALIZED) {
 		PIPE_UNLOCK(cpipe);
 #ifdef MAC
 		mac_pipe_destroy(pp);
@@ -1543,7 +1552,7 @@ pipe_kqfilter(struct file *fp, struct knote *kn)
 		break;
 	case EVFILT_WRITE:
 		kn->kn_fop = &pipe_wfiltops;
-		if (!cpipe->pipe_peer->pipe_present) {
+		if (cpipe->pipe_peer->pipe_present != PIPE_ACTIVE) {
 			/* other end of pipe has been closed */
 			PIPE_UNLOCK(cpipe);
 			return (EPIPE);
@@ -1586,7 +1595,8 @@ filt_piperead(struct knote *kn, long hint)
 		kn->kn_data = rpipe->pipe_map.cnt;
 
 	if ((rpipe->pipe_state & PIPE_EOF) ||
-	    (!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
+	    wpipe->pipe_present != PIPE_ACTIVE ||
+	    (wpipe->pipe_state & PIPE_EOF)) {
 		kn->kn_flags |= EV_EOF;
 		PIPE_UNLOCK(rpipe);
 		return (1);
@@ -1604,7 +1614,8 @@ filt_pipewrite(struct knote *kn, long hint)
 	struct pipe *wpipe = rpipe->pipe_peer;
 
 	PIPE_LOCK(rpipe);
-	if ((!wpipe->pipe_present) || (wpipe->pipe_state & PIPE_EOF)) {
+	if (wpipe->pipe_present != PIPE_ACTIVE ||
+	    (wpipe->pipe_state & PIPE_EOF)) {
 		kn->kn_data = 0;
 		kn->kn_flags |= EV_EOF;
 		PIPE_UNLOCK(rpipe);
diff --git a/sys/sys/pipe.h b/sys/sys/pipe.h
index 1d3729fc38c..2371b2a4e96 100644
--- a/sys/sys/pipe.h
+++ b/sys/sys/pipe.h
@@ -114,6 +114,13 @@ struct pipe {
 	int	pipe_present;		/* still present? */
 };
 
+/*
+ * Values for the pipe_present.
+ */
+#define PIPE_ACTIVE		1
+#define	PIPE_CLOSING		2
+#define	PIPE_FINALIZED		3
+
 /*
  * Container structure to hold the two pipe endpoints, mutex, and label
  * pointer.