DF-4382 / fix.diff
--- 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); |