⬢ DragonFlyBSD Kernel Audit
DF-5740 / fix.diff
← back to finding ↓ download raw
--- a/sys/netbt/hci_socket.c
+++ b/sys/netbt/hci_socket.c
@@ -426,16 +426,29 @@
 }
 
 /*
- * When command packet reaches the device, we can drop
- * it from the socket buffer (called from hci_output_acl)
+ * When command packet reaches the device, we can drop it from the
+ * socket buffer (called from hci_output_cmd, which may run on any
+ * CPU/netisr port, not on the socket's protocol port).
+ *
+ * Fix (DF-5740): touch so_snd only while holding the send sockbuf
+ * token, serializing against hci_send()/sosend; and drop the
+ * ctx-owned socket reference taken in hci_send() so the socket
+ * cannot be used after hci_sdetach() freed it.
  */
 void
 hci_drop(void *arg)
 {
 	struct socket *so = arg;
 
+	if (so == NULL)
+		return;
+
+	lwkt_gettoken(&so->so_snd.ssb_token);
 	sbdroprecord(&so->so_snd.sb);
 	sowwakeup(so);
+	lwkt_reltoken(&so->so_snd.ssb_token);
+
+	sofree(so);		/* drop the ctx-owned reference */
 }
 
 /*
@@ -453,13 +466,31 @@
 	DPRINTF("flushing %p\n", so);
 
 	TAILQ_FOREACH(unit, &hci_unit_list, hci_next) {
-		IF_POLL(&unit->hci_cmdwait, m);
-		while (m != NULL) {
-			ctx = M_GETCTX(m, struct socket *);
-			if (ctx == so)
-				M_SETCTX(m, NULL);
+		for (;;) {
+			struct socket *dead = NULL;
 
-			m = m->m_nextpkt;
+			/*
+			 * Fix (DF-5740): consume the ctx under the device
+			 * lock so the completion path (hci_output_cmd) and
+			 * this flush agree on exactly one consumer per
+			 * mbuf; whoever NULLs the ctx owns the callback
+			 * (and its socket reference).
+			 */
+			lockmgr(&unit->hci_devlock, LK_EXCLUSIVE);
+			IF_POLL(&unit->hci_cmdwait, m);
+			while (m != NULL) {
+				ctx = M_GETCTX(m, struct socket *);
+				if (ctx == so) {
+					M_SETCTX(m, NULL);
+					dead = ctx;
+					break;
+				}
+				m = m->m_nextpkt;
+			}
+			lockmgr(&unit->hci_devlock, LK_RELEASE);
+			if (dead == NULL)
+				break;
+			sofree(dead);	/* drop the ctx-owned reference */
 		}
 	}
 }
@@ -519,7 +550,16 @@
 		err = ENOMEM;
 		goto bad;
 	}
+	lwkt_gettoken(&pcb->hp_socket->so_snd.ssb_token);
 	sbappendrecord(&pcb->hp_socket->so_snd.sb, m0);
+	lwkt_reltoken(&pcb->hp_socket->so_snd.ssb_token);
+
+	/*
+	 * Fix (DF-5740): the completion callback context OWNS a socket
+	 * reference so that hci_drop() cannot run against a socket
+	 * that hci_sdetach() already freed.
+	 */
+	soreference(pcb->hp_socket);
 	M_SETCTX(m, pcb->hp_socket);	/* enable drop callback */
 
 	DPRINTFN(2, "(%s) opcode (%03x|%04x)\n",
@@ -1001,12 +1041,24 @@
 		 * copy to socket
 		 */
 		m0 = m_copym(m, 0, M_COPYALL, M_NOWAIT);
-		if (m0 && sbappendaddr(&pcb->hp_socket->so_rcv.sb,
-				(struct sockaddr *)&sa, m0, ctlmsg)) {
-			sorwakeup(pcb->hp_socket);
+		if (m0) {
+			/*
+			 * Fix (DF-5740): we run on a netisr port, not the
+			 * socket's protocol port; hold the receive sockbuf
+			 * token so this cannot interleave with a user
+			 * thread inside soreceive() on the same sockbuf.
+			 */
+			lwkt_gettoken(&pcb->hp_socket->so_rcv.ssb_token);
+			if (sbappendaddr(&pcb->hp_socket->so_rcv.sb,
+					(struct sockaddr *)&sa, m0, ctlmsg)) {
+				sorwakeup(pcb->hp_socket);
+			} else {
+				m_freem(ctlmsg);
+				m_freem(m0);
+			}
+			lwkt_reltoken(&pcb->hp_socket->so_rcv.ssb_token);
 		} else {
 			m_freem(ctlmsg);
-			m_freem(m0);
 		}
 	}
 }
--- a/sys/netbt/hci_unit.c
+++ b/sys/netbt/hci_unit.c
@@ -501,8 +501,16 @@
 	/*
 	 * If context is set, this was from a HCI raw socket
 	 * and a record needs to be dropped from the sockbuf.
+	 *
+	 * Fix (DF-5740): consume the context under the device lock so
+	 * that hci_cmdwait_flush() (socket teardown) and this
+	 * completion path agree on exactly one consumer per mbuf:
+	 * whoever NULLs the ctx pointer owns the callback.
 	 */
+	lockmgr(&unit->hci_devlock, LK_EXCLUSIVE);
 	arg = M_GETCTX(m, void *);
+	M_SETCTX(m, NULL);
+	lockmgr(&unit->hci_devlock, LK_RELEASE);
 	if (arg != NULL)
 		hci_drop(arg);