DF-0745 / fix.diff
diff --git a/sys/netbt/l2cap_misc.c b/sys/netbt/l2cap_misc.c index 0000000..1111111 100644 --- a/sys/netbt/l2cap_misc.c +++ b/sys/netbt/l2cap_misc.c @@ -165,9 +165,24 @@ { struct hci_link *link = req->lr_link; - callout_stop(&req->lr_rtx); - if (callout_active(&req->lr_rtx)) - return; + /* + * Synchronously drain any in-progress RTX callback before unlinking + * and freeing req. The previous code used callout_stop() plus a + * callout_active() guard, but that guard was dead code: callout_stop() + * (and callout_drain()) unconditionally clears CALLOUT_ACTIVE in + * _callout_cancel_or_stop() (kern_timeout.c), so the early return + * never fired. As a result l2cap_request_free() always fell through + * to the TAILQ_REMOVE/zfree even when the l2cap_rtx() callback was + * still in progress on another CPU, which raced the callback's own + * free of req and caused a double-free / TAILQ corruption. + * + * callout_drain() blocks until a running callback has returned + * (recursing from within the callback itself returns immediately), + * so an external caller can no longer race the callback. Combined + * with l2cap_rtx() no longer freeing req itself, req is now freed + * exactly once. + */ + callout_drain(&req->lr_rtx); TAILQ_REMOVE(&link->hl_reqs, req, lr_next); zfree(l2cap_req_pool, req); @@ -185,15 +200,27 @@ { struct l2cap_req *req = arg; struct l2cap_channel *chan; + uint8_t id; + /* + * The callback must not free req itself. l2cap_request_free() now + * callout_drain()s lr_rtx; calling it from inside the callback would + * recurse, and -- more importantly -- freeing req from the callback + * raced any concurrent external caller of l2cap_request_free() and + * produced a double-free. Capture the fields we still need (the old + * code also dereferenced req->lr_id in the DPRINTF *after* freeing + * req, a latent use-after-free), then let the channel close path + * clean up: l2cap_close() sweeps hl_reqs and frees any request whose + * lr_chan matches (l2cap_lower.c), so req is freed exactly once from + * a single context. + */ chan = req->lr_chan; - l2cap_request_free(req); + id = req->lr_id; - DPRINTF("cid %d, ident %d\n", (chan ? chan->lc_lcid : 0), req->lr_id); + DPRINTF("cid %d, ident %d\n", (chan ? chan->lc_lcid : 0), id); if (chan && chan->lc_state != L2CAP_CLOSED) l2cap_close(chan, ETIMEDOUT); - } /* |