⬢ DragonFlyBSD Kernel Audit
DF-4381 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/kern_dmsg.c
+++ b/sys/kern/kern_dmsg.c
@@ -1072,6 +1072,21 @@
 		kdio_printf(iocom, 1,
 			    "msgrx: state=%p error %d\n",
 			    state, error);
+	} else if ((msg->any.head.cmd & DMSGF_DELETE) &&
+		   (state->rxcmd & DMSGF_DELETE)) {
+		/*
+		 * Duplicate DELETE for a transaction whose receive side
+		 * is already closed.  Our own terminating reply may still
+		 * be queued for the transmit thread, so the state can
+		 * still be found here.  The old KKASSERT made this a
+		 * remote-triggerable panic; on kernels built without
+		 * KKASSERT the state callback would instead be dispatched
+		 * a second time (xdisk double-dispatch -> NULL derefs).
+		 * Drop the duplicate instead.
+		 */
+		kdio_printf(iocom, 1, "%s\n",
+			    "msgrx: duplicate DELETE dropped");
+		error = EALREADY;
 	} else if (msg->any.head.cmd & DMSGF_DELETE) {
 		KKASSERT((state->rxcmd & DMSGF_DELETE) == 0);
 		state->rxcmd |= DMSGF_DELETE;
--- a/sys/dev/disk/xdisk/xdisk.c
+++ b/sys/dev/disk/xdisk/xdisk.c
@@ -535,6 +535,15 @@
 		sc = msg->state->any.xa_sc;
 		xa_printf(3, "LINK_SPAN state %p delete for %s (sc=%p)\n",
 			  msg->state, (sc ? sc->pfs_label : "(null)"), sc);
+		if (sc == NULL) {
+			/*
+			 * Duplicate or raced DELETE: this span was already
+			 * removed from its softc.  Do not deref sc and do
+			 * not double-remove from spanq.
+			 */
+			kdmsg_msg_reply(msg, 0);
+			break;
+		}
 		lockmgr(&sc->lk, LK_EXCLUSIVE);
 		msg->state->any.xa_sc = NULL;
 		TAILQ_REMOVE(&sc->spanq, msg->state, user_entry);
@@ -1124,10 +1133,27 @@
 	 * (see xa_close() for other interactions)
 	 */
 	if (msg->any.head.cmd & DMSGF_DELETE) {
-		kdmsg_state_reply(tag->state, 0);
+		kdmsg_state_t *tstate = tag->state;
+
+		/*
+		 * Duplicate DELETE hardening: tag->state is NULLed by
+		 * xa_done(); a (raced) second dispatch must not feed a
+		 * NULL state to kdmsg_state_reply().
+		 */
+		if (tstate)
+			kdmsg_state_reply(tstate, 0);
 		if (sc->open_tag == tag) {
 			sc->open_tag = NULL;
 			xa_done(tag, 0);
+			/*
+			 * Nobody will xa_wait() on this tag (open_tag is
+			 * now NULL so xa_close() skips it) and async == 0,
+			 * so without an explicit release the tag would be
+			 * stranded on tag_pendq forever (DF-4382).
+			 */
+			lockmgr(&sc->lk, LK_RELEASE);
+			xa_release(tag, 0);
+			lockmgr(&sc->lk, LK_EXCLUSIVE);
 		} else {
 			tag->async = 0;
 			xa_done(tag, 0);