From e649887b1e67b5fdc489ebf9e42579754201625c Mon Sep 17 00:00:00 2001
From: Alfred Perlstein <alfred@FreeBSD.org>
Date: Mon, 6 May 2002 19:31:28 +0000
Subject: [PATCH] Make funsetown() take a 'struct sigio **' so that the locking
 can be done internally.

Ensure that no one can fsetown() to a dying process/pgrp.  We need
to check the process for P_WEXIT to see if it's exiting.  Process
groups are already safe because there is no such thing as a pgrp
zombie, therefore the proctree lock completely protects the pgrp
from having sigio structures associated with it after it runs
funsetownlst.

Add sigio lock to witness list under proctree and allproc, but over
proc and pgrp.

Seigo Tanimura helped with this.
---
 sys/dev/drm/drm_drv.h   |  2 +-
 sys/kern/kern_descrip.c | 71 ++++++++++++++++++++++++++---------------
 sys/kern/kern_exit.c    |  4 ---
 sys/kern/kern_proc.c    |  5 +--
 sys/kern/subr_log.c     |  2 +-
 sys/kern/subr_witness.c |  1 +
 sys/kern/sys_pipe.c     |  2 +-
 sys/kern/tty.c          |  2 +-
 sys/kern/uipc_socket.c  |  2 +-
 sys/net/bpf.c           |  2 +-
 sys/net/if_tap.c        |  2 +-
 sys/net/if_tun.c        |  2 +-
 sys/sys/filedesc.h      |  2 +-
 13 files changed, 56 insertions(+), 43 deletions(-)

diff --git a/sys/dev/drm/drm_drv.h b/sys/dev/drm/drm_drv.h
index 952449923a6..701395519fc 100644
--- a/sys/dev/drm/drm_drv.h
+++ b/sys/dev/drm/drm_drv.h
@@ -1098,7 +1098,7 @@ int DRM( close)(dev_t kdev, int flags, int fmt, DRM_OS_STRUCTPROC *p)
 	DRM(fasync)( -1, filp, 0 );
 #endif /* __linux__ */
 #ifdef __FreeBSD__
-	funsetown(dev->buf_sigio);
+	funsetown(&dev->buf_sigio);
 #endif /* __FreeBSD__ */
 
 	DRM_OS_LOCK;
diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index 3e15012d695..e03c09f1a44 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -534,17 +534,18 @@ do_dup(fdp, old, new, retval, td)
  * free sigio.
  */
 void
-funsetown(sigio)
-	struct sigio *sigio;
+funsetown(sigiop)
+	struct sigio **sigiop;
 {
+	struct sigio *sigio;
 
 	SIGIO_LOCK();
+	sigio = *sigiop;
 	if (sigio == NULL) {
 		SIGIO_UNLOCK();
 		return;
 	}
 	*(sigio->sio_myref) = NULL;
-	SIGIO_UNLOCK();
 	if ((sigio)->sio_pgid < 0) {
 		struct pgrp *pg = (sigio)->sio_pgrp;
 		PGRP_LOCK(pg);
@@ -558,11 +559,17 @@ funsetown(sigio)
 			     sigio, sio_pgsigio);
 		PROC_UNLOCK(p);
 	}
+	SIGIO_UNLOCK();
 	crfree(sigio->sio_ucred);
 	FREE(sigio, M_SIGIO);
 }
 
-/* Free a list of sigio structures. */
+/*
+ * Free a list of sigio structures.
+ * We only need to lock the SIGIO_LOCK because we have made ourselves
+ * inaccessable to callers of fsetown and therefore do not need to lock
+ * the proc or pgrp struct for the list manipulation.
+ */
 void
 funsetownlst(sigiolst)
 	struct sigiolst *sigiolst;
@@ -571,8 +578,6 @@ funsetownlst(sigiolst)
 	struct proc *p;
 	struct pgrp *pg;
 
-	SIGIO_ASSERT(MA_OWNED);
-
 	sigio = SLIST_FIRST(sigiolst);
 	if (sigio == NULL)
 		return;
@@ -586,36 +591,40 @@ funsetownlst(sigiolst)
 	 */
 	if (sigio->sio_pgid < 0) {
 		pg = sigio->sio_pgrp;
-		PGRP_LOCK_ASSERT(pg, MA_OWNED);
+		PGRP_LOCK_ASSERT(pg, MA_NOTOWNED);
 	} else /* if (sigio->sio_pgid > 0) */ {
 		p = sigio->sio_proc;
-		PROC_LOCK_ASSERT(p, MA_OWNED);
+		PROC_LOCK_ASSERT(p, MA_NOTOWNED);
 	}
 
+	SIGIO_LOCK();
 	while ((sigio = SLIST_FIRST(sigiolst)) != NULL) {
 		*(sigio->sio_myref) = NULL;
 		if (pg != NULL) {
-			KASSERT(sigio->sio_pgid < 0, ("Proc sigio in pgrp sigio list"));
-			KASSERT(sigio->sio_pgrp == pg, ("Bogus pgrp in sigio list"));
-			SLIST_REMOVE(&pg->pg_sigiolst, sigio, sigio, sio_pgsigio);
-			PGRP_UNLOCK(pg);
-			SIGIO_UNLOCK();
-			crfree(sigio->sio_ucred);
-			FREE(sigio, M_SIGIO);
-			SIGIO_LOCK();
+			KASSERT(sigio->sio_pgid < 0,
+			    ("Proc sigio in pgrp sigio list"));
+			KASSERT(sigio->sio_pgrp == pg,
+			    ("Bogus pgrp in sigio list"));
 			PGRP_LOCK(pg);
+			SLIST_REMOVE(&pg->pg_sigiolst, sigio, sigio,
+			    sio_pgsigio);
+			PGRP_UNLOCK(pg);
 		} else /* if (p != NULL) */ {
-			KASSERT(sigio->sio_pgid > 0, ("Pgrp sigio in proc sigio list"));
-			KASSERT(sigio->sio_proc == p, ("Bogus proc in sigio list"));
-			SLIST_REMOVE(&p->p_sigiolst, sigio, sigio, sio_pgsigio);
-			PROC_UNLOCK(p);
-			SIGIO_UNLOCK();
-			crfree(sigio->sio_ucred);
-			FREE(sigio, M_SIGIO);
-			SIGIO_LOCK();
+			KASSERT(sigio->sio_pgid > 0,
+			    ("Pgrp sigio in proc sigio list"));
+			KASSERT(sigio->sio_proc == p,
+			    ("Bogus proc in sigio list"));
 			PROC_LOCK(p);
+			SLIST_REMOVE(&p->p_sigiolst, sigio, sigio,
+			    sio_pgsigio);
+			PROC_UNLOCK(p);
 		}
+		SIGIO_UNLOCK();
+		crfree(sigio->sio_ucred);
+		FREE(sigio, M_SIGIO);
+		SIGIO_LOCK();
 	}
+	SIGIO_UNLOCK();
 }
 
 /*
@@ -635,7 +644,7 @@ fsetown(pgid, sigiop)
 	int ret;
 
 	if (pgid == 0) {
-		funsetown(*sigiop);
+		funsetown(sigiop);
 		return (0);
 	}
 
@@ -693,9 +702,19 @@ fsetown(pgid, sigiop)
 
 		proc = NULL;
 	}
-	funsetown(*sigiop);
+	funsetown(sigiop);
 	if (pgid > 0) {
 		PROC_LOCK(proc);
+		/* 
+		 * since funsetownlst() is called without the proctree
+		 * locked we need to check for P_WEXIT.
+		 * XXX: is ESRCH correct?
+		 */
+		if ((proc->p_flag & P_WEXIT) != 0) {
+			PROC_UNLOCK(proc);
+			ret = ESRCH;
+			goto fail;
+		}
 		SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio);
 		sigio->sio_proc = proc;
 		PROC_UNLOCK(proc);
diff --git a/sys/kern/kern_exit.c b/sys/kern/kern_exit.c
index f23c7562747..ecaed6fa947 100644
--- a/sys/kern/kern_exit.c
+++ b/sys/kern/kern_exit.c
@@ -192,11 +192,7 @@ exit1(td, rv)
 	 * Reset any sigio structures pointing to us as a result of
 	 * F_SETOWN with our pid.
 	 */
-	SIGIO_LOCK();
-	PROC_LOCK(p);
 	funsetownlst(&p->p_sigiolst);
-	PROC_UNLOCK(p);
-	SIGIO_UNLOCK();
 
 	/*
 	 * Close open files and release open-file table.
diff --git a/sys/kern/kern_proc.c b/sys/kern/kern_proc.c
index 2dde10861ce..8f923319fa0 100644
--- a/sys/kern/kern_proc.c
+++ b/sys/kern/kern_proc.c
@@ -474,16 +474,13 @@ pgdelete(pgrp)
 	PGRP_LOCK_ASSERT(pgrp, MA_NOTOWNED);
 	SESS_LOCK_ASSERT(pgrp->pg_session, MA_NOTOWNED);
 
-	SIGIO_LOCK();
-	PGRP_LOCK(pgrp);
-
 	/*
 	 * Reset any sigio structures pointing to us as a result of
 	 * F_SETOWN with our pgid.
 	 */
 	funsetownlst(&pgrp->pg_sigiolst);
-	SIGIO_UNLOCK();
 
+	PGRP_LOCK(pgrp);
 	if (pgrp->pg_session->s_ttyp != NULL &&
 	    pgrp->pg_session->s_ttyp->t_pgrp == pgrp)
 		pgrp->pg_session->s_ttyp->t_pgrp = NULL;
diff --git a/sys/kern/subr_log.c b/sys/kern/subr_log.c
index 568b3fe128d..2c0156833d9 100644
--- a/sys/kern/subr_log.c
+++ b/sys/kern/subr_log.c
@@ -118,7 +118,7 @@ logclose(dev_t dev, int flag, int mode, struct thread *td)
 	log_open = 0;
 	callout_stop(&logsoftc.sc_callout);
 	logsoftc.sc_state = 0;
-	funsetown(logsoftc.sc_sigio);
+	funsetown(&logsoftc.sc_sigio);
 	return (0);
 }
 
diff --git a/sys/kern/subr_witness.c b/sys/kern/subr_witness.c
index 59cf2caab56..fcc9b53ec76 100644
--- a/sys/kern/subr_witness.c
+++ b/sys/kern/subr_witness.c
@@ -189,6 +189,7 @@ static struct witness_order_list_entry order_lists[] = {
 	{ "Giant", &lock_class_mtx_sleep },
 	{ "proctree", &lock_class_sx },
 	{ "allproc", &lock_class_sx },
+	{ "sigio lock", &lock_class_mtx_sleep },
 	{ "process group", &lock_class_mtx_sleep },
 	{ "process lock", &lock_class_mtx_sleep },
 	{ "session", &lock_class_mtx_sleep },
diff --git a/sys/kern/sys_pipe.c b/sys/kern/sys_pipe.c
index d0682fa0cdb..c79f9ec23a0 100644
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -1246,7 +1246,7 @@ pipe_close(fp, td)
 
 	fp->f_ops = &badfileops;
 	fp->f_data = NULL;
-	funsetown(cpipe->pipe_sigio);
+	funsetown(&cpipe->pipe_sigio);
 	pipeclose(cpipe);
 	return (0);
 }
diff --git a/sys/kern/tty.c b/sys/kern/tty.c
index 7b3459acc21..5c7719b06a8 100644
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -246,7 +246,7 @@ ttyclose(tp)
 {
 	int s;
 
-	funsetown(tp->t_sigio);
+	funsetown(&tp->t_sigio);
 	s = spltty();
 	if (constty == tp)
 		constty = NULL;
diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 33f8a4dd4c3..4bbc6e259ed 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -305,7 +305,7 @@ soclose(so)
 	int s = splnet();		/* conservative */
 	int error = 0;
 
-	funsetown(so->so_sigio);
+	funsetown(&so->so_sigio);
 	if (so->so_options & SO_ACCEPTCONN) {
 		struct socket *sp, *sonext;
 
diff --git a/sys/net/bpf.c b/sys/net/bpf.c
index 325af5430ce..14764981530 100644
--- a/sys/net/bpf.c
+++ b/sys/net/bpf.c
@@ -368,7 +368,7 @@ bpfclose(dev, flags, fmt, td)
 		callout_stop(&d->bd_callout);
 	d->bd_state = BPF_IDLE;
 	BPFD_UNLOCK(d);
-	funsetown(d->bd_sigio);
+	funsetown(&d->bd_sigio);
 	mtx_lock(&bpf_mtx);
 	if (d->bd_bif)
 		bpf_detachd(d);
diff --git a/sys/net/if_tap.c b/sys/net/if_tap.c
index 45ce21f0586..ef0a36d9958 100644
--- a/sys/net/if_tap.c
+++ b/sys/net/if_tap.c
@@ -491,7 +491,7 @@ tapclose(dev, foo, bar, td)
 		splx(s);
 	}
 
-	funsetown(tp->tap_sigio);
+	funsetown(&tp->tap_sigio);
 	selwakeup(&tp->tap_rsel);
 
 	tp->tap_flags &= ~TAP_OPEN;
diff --git a/sys/net/if_tun.c b/sys/net/if_tun.c
index a6c89fb01fb..18435fd6339 100644
--- a/sys/net/if_tun.c
+++ b/sys/net/if_tun.c
@@ -335,7 +335,7 @@ tunclose(dev_t dev, int foo, int bar, struct thread *td)
 		splx(s);
 	}
 
-	funsetown(tp->tun_sigio);
+	funsetown(&tp->tun_sigio);
 	selwakeup(&tp->tun_rsel);
 
 	TUNDEBUG ("%s%d: closed\n", ifp->if_name, ifp->if_unit);
diff --git a/sys/sys/filedesc.h b/sys/sys/filedesc.h
index d2d108c7439..ecda57418e2 100644
--- a/sys/sys/filedesc.h
+++ b/sys/sys/filedesc.h
@@ -156,7 +156,7 @@ void	ffree(struct file *fp);
 static __inline struct file *	fget_locked(struct filedesc *fdp, int fd);
 pid_t	fgetown(struct sigio *sigio);
 int	fsetown(pid_t pgid, struct sigio **sigiop);
-void	funsetown(struct sigio *sigio);
+void	funsetown(struct sigio **sigiop);
 void	funsetownlst(struct sigiolst *sigiolst);
 int	getvnode(struct filedesc *fdp, int fd, struct file **fpp);
 void	setugidsafety(struct thread *td);