fsetown: Fix process lookup bugs

- pget()/pfind() will acquire the PID hash bucket locks, which are
  sleepable sx locks, but this means that the sigio mutex cannot be held
  while calling these functions.  Instead, use pget() to hold the
  process, after which we lock the sigio and proc locks, respectively.
- funsetownlst() assumes that processes cannot be registered for SIGIO
  once they have P_WEXIT set.  However, pfind() will happily return
  exiting processes, breaking the invariant.  Add an explicit check for
  P_WEXIT in fsetown() to fix this. [1]

Fixes:	f52979098d3c ("Fix a pair of races in SIGIO registration")
Reported by:	syzkaller [1]
Reviewed by:	kib
Sponsored by:	The FreeBSD Foundation

(cherry picked from commit 1d874ba4f8ba58296cd9df611f5346dad8e91664)

1 files changed, 23 insertions, 13 deletions

diff --git a/sys/kern/kern_descrip.c b/sys/kern/kern_descrip.c
index c7269e4b33a9..e6a6a36801e4 100644
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1031,18 +1031,16 @@ funsetown_locked(struct sigio *sigio)
 
 	if (sigio == NULL)
 		return (NULL);
-	*(sigio->sio_myref) = NULL;
+	*sigio->sio_myref = NULL;
 	if (sigio->sio_pgid < 0) {
 		pg = sigio->sio_pgrp;
 		PGRP_LOCK(pg);
-		SLIST_REMOVE(&sigio->sio_pgrp->pg_sigiolst, sigio,
-		    sigio, sio_pgsigio);
+		SLIST_REMOVE(&pg->pg_sigiolst, sigio, sigio, sio_pgsigio);
 		PGRP_UNLOCK(pg);
 	} else {
 		p = sigio->sio_proc;
 		PROC_LOCK(p);
-		SLIST_REMOVE(&sigio->sio_proc->p_sigiolst, sigio,
-		    sigio, sio_pgsigio);
+		SLIST_REMOVE(&p->p_sigiolst, sigio, sigio, sio_pgsigio);
 		PROC_UNLOCK(p);
 	}
 	return (sigio);
@@ -1156,18 +1154,25 @@ fsetown(pid_t pgid, struct sigio **sigiop)
 	}
 
 	ret = 0;
+	osigio = NULL;
 
 	sigio = malloc(sizeof(struct sigio), M_SIGIO, M_WAITOK);
 	sigio->sio_pgid = pgid;
 	sigio->sio_ucred = crhold(curthread->td_ucred);
 	sigio->sio_myref = sigiop;
 
-	sx_slock(&proctree_lock);
-	SIGIO_LOCK();
-	osigio = funsetown_locked(*sigiop);
 	if (pgid > 0) {
-		proc = pfind(pgid);
-		if (proc == NULL) {
+		ret = pget(pgid, PGET_NOTWEXIT | PGET_NOTID | PGET_HOLD, &proc);
+		SIGIO_LOCK();
+		if (ret != 0)
+			goto fail;
+
+		osigio = funsetown_locked(*sigiop);
+
+		PROC_LOCK(proc);
+		_PRELE(proc);
+		if ((proc->p_flag & P_WEXIT) != 0) {
+			PROC_UNLOCK(proc);
 			ret = ESRCH;
 			goto fail;
 		}
@@ -1190,12 +1195,17 @@ fsetown(pid_t pgid, struct sigio **sigiop)
 		SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio);
 		PROC_UNLOCK(proc);
 	} else /* if (pgid < 0) */ {
+		sx_slock(&proctree_lock);
+		SIGIO_LOCK();
 		pgrp = pgfind(-pgid);
 		if (pgrp == NULL) {
+			sx_sunlock(&proctree_lock);
 			ret = ESRCH;
 			goto fail;
 		}
 
+		osigio = funsetown_locked(*sigiop);
+
 		/*
 		 * Policy - Don't allow a process to FSETOWN a process
 		 * in another session.
@@ -1205,16 +1215,17 @@ fsetown(pid_t pgid, struct sigio **sigiop)
 		 * group for maximum safety.
 		 */
 		if (pgrp->pg_session != curthread->td_proc->p_session) {
+			sx_sunlock(&proctree_lock);
 			PGRP_UNLOCK(pgrp);
 			ret = EPERM;
 			goto fail;
 		}
 
-		SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio);
 		sigio->sio_pgrp = pgrp;
+		SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio);
 		PGRP_UNLOCK(pgrp);
+		sx_sunlock(&proctree_lock);
 	}
-	sx_sunlock(&proctree_lock);
 	*sigiop = sigio;
 	SIGIO_UNLOCK();
 	if (osigio != NULL)
@@ -1223,7 +1234,6 @@ fsetown(pid_t pgid, struct sigio **sigiop)
 
 fail:
 	SIGIO_UNLOCK();
-	sx_sunlock(&proctree_lock);
 	sigiofree(sigio);
 	if (osigio != NULL)
 		sigiofree(osigio);