DF-5825 / fix.diff
diff --git a/sys/kern/kern_dmsg.c b/sys/kern/kern_dmsg.c index d6e90ee..0678b80 100644 --- a/sys/kern/kern_dmsg.c +++ b/sys/kern/kern_dmsg.c @@ -983,6 +983,25 @@ again: error = EINVAL; break; } + /* + * A received REPLY can only be for a transaction that WE + * initiated, which is tracked in statewr_tree. The tree + * used for the initial lookup is selected solely by the + * attacker-controlled DMSGF_REVTRANS bit, so without this + * check a peer can deliver a REPLY|CREATE message against + * a state it initiated (staterd_tree, KDMSG_STATE_OPPOSITE) + * and inject DMSGF_REPLY into rxcmd. The subsequent DELETE + * processing would then RB_REMOVE() the state from the + * WRONG red-black tree, leaving a dangling tree root that + * points at freed memory (use-after-free in every later + * tree operation on the link). + */ + if (state->flags & KDMSG_STATE_OPPOSITE) { + kdio_printf(iocom, 1, "%s\n", + "msgrx: REPLY on received state"); + error = EINVAL; + break; + } state->rxcmd = msg->any.head.cmd & ~DMSGF_DELETE; error = 0; break; @@ -1003,6 +1022,25 @@ again: break; } + /* + * Same protection as the REPLY|CREATE case above: a reply + * can only be for a transaction WE initiated. Without + * this check a REPLY|DELETE against a received state runs + * into the wrong-branch KKASSERT (or the wrong tree) in + * the DELETE post-processing below. + */ + if (state->flags & KDMSG_STATE_OPPOSITE) { + if (msg->any.head.cmd & DMSGF_ABORT) { + error = EALREADY; + } else { + kdio_printf(iocom, 1, "%s\n", + "msgrx: REPLY|DELETE on " + "received state"); + error = EINVAL; + } + break; + } + /* * Received REPLY+ABORT+DELETE in case where msgid has * already been reused for an unrelated message, @@ -1077,7 +1115,12 @@ done: state->rxcmd |= DMSGF_DELETE; if (state->txcmd & DMSGF_DELETE) { KKASSERT(state->flags & KDMSG_STATE_RBINSERTED); - if (state->rxcmd & DMSGF_REPLY) { + /* + * Defense-in-depth: select the tree by which side + * the state is on (KDMSG_STATE_OPPOSITE), not by + * bits in rxcmd which a malicious peer can smear. + */ + if ((state->flags & KDMSG_STATE_OPPOSITE) == 0) { KKASSERT(msg->any.head.cmd & DMSGF_REPLY); RB_REMOVE(kdmsg_state_tree, |