DF-5740 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 | --- 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); |