DragonFlyBSD Kernel Audit
DF-2682 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/kern_descrip.c
+++ b/sys/kern/kern_descrip.c
@@ -1,19 +1,37 @@
 void
 funsetown(struct sigio **sigiop)
 {
+	struct sigio *sigio;
+
+	/*
+	 * DF-2682 FIX: the pointer is loaded and cleared under
+	 * sigio_token.  Raw-pointer readers (the pgsigio() call sites)
+	 * hold sigio_token shared around their load+use;
+	 * funsetown_free() re-acquires it exclusively for the kfree()
+	 * so the free is ordered after the last in-flight reader.
+	 */
+	lwkt_gettoken(&sigio_token);
+	if ((sigio = *sigiop) == NULL) {
+		lwkt_reltoken(&sigio_token);
+		return;
+	}
+	KKASSERT(sigiop == sigio->sio_myref);
+	*sigiop = NULL;
+	lwkt_reltoken(&sigio_token);
+
+	funsetown_free(sigio);
+}
+
+/*
+ * Tear down and free a sigio whose *sio_myref has already been cleared.
+ * No sigio_token is held across the owner-list removals (exit1() holds
+ * p_token across funsetownlst(), so nesting the tokens can livelock).
+ */
+static void
+funsetown_free(struct sigio *sigio)
+{
 	struct pgrp *pgrp;
 	struct proc *p;
-	struct sigio *sigio;
-
-	if ((sigio = *sigiop) != NULL) {
-		lwkt_gettoken(&sigio_token);	/* protect sigio */
-		KKASSERT(sigiop == sigio->sio_myref);
-		sigio = *sigiop;
-		*sigiop = NULL;
-		lwkt_reltoken(&sigio_token);
-	}
-	if (sigio == NULL)
-		return;
 
 	if (sigio->sio_pgid < 0) {
 		pgrp = sigio->sio_pgrp;
@@ -22,7 +40,7 @@
 		SLIST_REMOVE(&pgrp->pg_sigiolst, sigio, sigio, sio_pgsigio);
 		lwkt_reltoken(&pgrp->pg_token);
 		pgrel(pgrp);
-	} else /* if ((*sigiop)->sio_pgid > 0) */ {
+	} else /* if (sigio->sio_pgid > 0) */ {
 		p = sigio->sio_proc;
 		sigio->sio_proc = NULL;
 		PHOLD(p);
@@ -33,20 +51,46 @@
 	}
 	crfree(sigio->sio_ucred);
 	sigio->sio_ucred = NULL;
+
+	/*
+	 * DF-2682 FIX: free only after every in-flight reader is done.
+	 */
+	lwkt_gettoken(&sigio_token);
 	kfree(sigio, M_SIGIO);
+	lwkt_reltoken(&sigio_token);
 }
 
 /*
  * Free a list of sigio structures.  Caller is responsible for ensuring
  * that the list is MPSAFE.
+ *
+ * DF-2683 FIX (v4): read the list head UNDER sigio_token (a head read
+ * outside the token raced free+slab-reuse into an identity confusion
+ * and a wrong-list SLIST_REMOVE).  Publication-in-flight entries are
+ * retried after a 1-tick sleep; sleeping releases our lwkt tokens so
+ * the publisher/remover can always run.
  */
 void
 funsetownlst(struct sigiolst *sigiolst)
 {
 	struct sigio *sigio;
 
-	while ((sigio = SLIST_FIRST(sigiolst)) != NULL)
-		funsetown(sigio->sio_myref);
+	for (;;) {
+		lwkt_gettoken(&sigio_token);
+		sigio = SLIST_FIRST(sigiolst);
+		if (sigio == NULL) {
+			lwkt_reltoken(&sigio_token);
+			break;
+		}
+		if (*sigio->sio_myref != sigio) {
+			lwkt_reltoken(&sigio_token);
+			tsleep(sigio, 0, "sigbrth", 1);
+			continue;
+		}
+		*sigio->sio_myref = NULL;
+		lwkt_reltoken(&sigio_token);
+		funsetown_free(sigio);
+	}
 }
 
 /*
@@ -108,25 +152,33 @@
 		}
 	}
 	sigio = kmalloc(sizeof(struct sigio), M_SIGIO, M_WAITOK | M_ZERO);
-	if (pgid > 0) {
-		KKASSERT(pgrp == NULL);
-		lwkt_gettoken(&proc->p_token);
-		SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio);
-		sigio->sio_proc = proc;
-		lwkt_reltoken(&proc->p_token);
-	} else {
-		KKASSERT(proc == NULL);
-		lwkt_gettoken(&pgrp->pg_token);
-		SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio);
-		sigio->sio_pgrp = pgrp;
-		lwkt_reltoken(&pgrp->pg_token);
-		pgrp = NULL;
-	}
+
+	/*
+	 * DF-2683 FIX: fully initialize the sigio BEFORE linking it into
+	 * the owner's list (old code initialized sio_pgid/sio_ucred/
+	 * sio_ruid/sio_myref only after the insert, letting a teardown
+	 * walker consume a half-born sigio -> funsetown(NULL) NULL fault
+	 * or wrong-branch SLIST_REMOVE walk-off).
+	 */
 	sigio->sio_pgid = pgid;
 	sigio->sio_ucred = crhold(curthread->td_ucred);
 	/* It would be convenient if p_ruid was in ucred. */
 	sigio->sio_ruid = sigio->sio_ucred->cr_ruid;
 	sigio->sio_myref = sigiop;
+	if (pgid > 0) {
+		KKASSERT(pgrp == NULL);
+		sigio->sio_proc = proc;
+		lwkt_gettoken(&proc->p_token);
+		SLIST_INSERT_HEAD(&proc->p_sigiolst, sigio, sio_pgsigio);
+		lwkt_reltoken(&proc->p_token);
+	} else {
+		KKASSERT(proc == NULL);
+		sigio->sio_pgrp = pgrp;
+		lwkt_gettoken(&pgrp->pg_token);
+		SLIST_INSERT_HEAD(&pgrp->pg_sigiolst, sigio, sio_pgsigio);
+		lwkt_reltoken(&pgrp->pg_token);
+		pgrp = NULL;
+	}
 
 	lwkt_gettoken(&sigio_token);
 	while (*sigiop)

--- a/sys/kern/uipc_socket2.c
+++ b/sys/kern/uipc_socket2.c
@@ -598,8 +598,13 @@
 	/*
 	 * Misc other events
 	 */
-	if ((so->so_state & SS_ASYNC) && so->so_sigio != NULL)
-		pgsigio(so->so_sigio, SIGIO, 0);
+	/* DF-2682 FIX: load+use the sigio under sigio_token (shared) */
+	if (so->so_state & SS_ASYNC) {
+		lwkt_gettoken_shared(&sigio_token);
+		if (so->so_sigio != NULL)
+			pgsigio(so->so_sigio, SIGIO, 0);
+		lwkt_reltoken(&sigio_token);
+	}
 	if (ssb->ssb_flags & SSB_UPCALL)
 		(*so->so_upcall)(so, so->so_upcallarg, M_NOWAIT);
 	KNOTE(&ssb->ssb_kq.ki_note, 0);
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -2534,8 +2534,13 @@
 void
 sohasoutofband(struct socket *so)
 {
-	if (so->so_sigio != NULL)
-		pgsigio(so->so_sigio, SIGURG, 0);
+	/* DF-2682 FIX: load+use the sigio under sigio_token (shared) */
+	{
+		lwkt_gettoken_shared(&sigio_token);
+		if (so->so_sigio != NULL)
+			pgsigio(so->so_sigio, SIGURG, 0);
+		lwkt_reltoken(&sigio_token);
+	}
 	/*
 	 * NOTE:
 	 * There is no need to use NOTE_OOB as KNOTE hint here:
--- a/sys/kern/kern_event.c
+++ b/sys/kern/kern_event.c
@@ -2051,8 +2051,13 @@
 	/*
 	 * Send SIGIO on request (typically set up as a mailbox signal)
 	 */
-	if (kq->kq_sigio && (kq->kq_state & KQ_ASYNC) && kq->kq_count == 1)
-		pgsigio(kq->kq_sigio, SIGIO, 0);
+	/* DF-2682 FIX: load+use the sigio under sigio_token (shared) */
+	if ((kq->kq_state & KQ_ASYNC) && kq->kq_count == 1) {
+		lwkt_gettoken_shared(&sigio_token);
+		if (kq->kq_sigio)
+			pgsigio(kq->kq_sigio, SIGIO, 0);
+		lwkt_reltoken(&sigio_token);
+	}
 
 	kqueue_wakeup(kq);
 }
--- a/sys/kern/sys_pipe.c
+++ b/sys/kern/sys_pipe.c
@@ -208,9 +208,11 @@
 static __inline void
 pipewakeup(struct pipebuf *pb, int dosigio)
 {
-	if (dosigio && (pb->state & PIPE_ASYNC) && pb->sigio) {
+	/* DF-2682 FIX: load the sigio inside the token too */
+	if (dosigio && (pb->state & PIPE_ASYNC)) {
 		lwkt_gettoken(&sigio_token);
-		pgsigio(pb->sigio, SIGIO, 0);
+		if (pb->sigio)
+			pgsigio(pb->sigio, SIGIO, 0);
 		lwkt_reltoken(&sigio_token);
 	}
 	KNOTE(&pb->kq.ki_note, 0);
--- a/sys/kern/tty.c
+++ b/sys/kern/tty.c
@@ -2363,7 +2363,11 @@
 {
 	lwkt_gettoken(&tp->t_token);
 	if (ISSET(tp->t_state, TS_ASYNC) && tp->t_sigio != NULL)
-		pgsigio(tp->t_sigio, SIGIO, (tp->t_session != NULL));
+		/* DF-2682 FIX: load+use the sigio under sigio_token */
+		lwkt_gettoken_shared(&sigio_token);
+		if (tp->t_sigio)
+			pgsigio(tp->t_sigio, SIGIO, (tp->t_session != NULL));
+		lwkt_reltoken(&sigio_token);
 	wakeup(TSA_HUP_OR_INPUT(tp));
 	KNOTE(&tp->t_rkq.ki_note, 0);
 	lwkt_reltoken(&tp->t_token);
@@ -2377,7 +2381,11 @@
 {
 	lwkt_gettoken(&tp->t_token);
 	if (ISSET(tp->t_state, TS_ASYNC) && tp->t_sigio != NULL)
-		pgsigio(tp->t_sigio, SIGIO, (tp->t_session != NULL));
+		/* DF-2682 FIX: load+use the sigio under sigio_token */
+		lwkt_gettoken_shared(&sigio_token);
+		if (tp->t_sigio)
+			pgsigio(tp->t_sigio, SIGIO, (tp->t_session != NULL));
+		lwkt_reltoken(&sigio_token);
 	if (ISSET(tp->t_state, TS_BUSY | TS_SO_OCOMPLETE) ==
 	    TS_SO_OCOMPLETE && tp->t_outq.c_cc == 0) {
 		CLR(tp->t_state, TS_SO_OCOMPLETE);