bsd/callout: treat a null callout in callout_reset_on as a no-op - #1486
bsd/callout: treat a null callout in callout_reset_on as a no-op#1486gburd wants to merge 2 commits into
Conversation
callout_reset_on() dereferences its callout pointer immediately. Under concurrency the FreeBSD ARP/link-layer path can reach callout_reset() on a link-layer entry (la->la_timer) that has raced with its own teardown, passing a null callout. The dereference then page-faults at address 0 deep in the network output path (arpresolve -> ether_output -> tcp_output), aborting the instance instead of harmlessly doing nothing. Guard the entry: if the callout pointer is null there is nothing to arm, so return 0 (no pending callout deactivated), matching the contract that resetting an already-inactive callout is a no-op. Signed-off-by: Greg Burd <greg@burd.me>
…andler The callout dispatcher (_callout_thread) invoked a callout's handler while the callout was still linked in the ordered set that callout_compare keys on. Handlers on the TCP and link-layer teardown paths free their callout, so the set was left holding a dangling pointer; the next add_callout insert then dereferenced freed memory in callout_compare and page-faulted at address 0, aborting the guest under sustained load (a large working-set PostgreSQL run churns many concurrent TCP and ZFS timers). Remove the callout from the set before dropping the lock to run the handler. If the handler reschedules via callout_reset, add_callout re-links it, so a post-handler have_callout check now cleanly means rescheduled. Rework the post-handler completion accordingly and preserve callout_drain semantics: a drain waiter is always completed and woken (whether or not the handler re-armed the callout), so drain cannot hang. Also guard the dispatcher against a null head from get_one() when the set drains between the wait predicate and the dequeue. This is the root cause behind the intermittent arpresolve/tcp_output callout crash that the earlier callout_reset_on null-guard only partially masked. Validated: a 500-warehouse PostgreSQL schema build (whose index phase previously aborted here) now completes and survives reboots.
|
Added a second commit that fixes the root cause behind this crash, not just the symptom. The dispatcher ( The fix removes the callout from the set before dropping the lock to run the handler; if the handler reschedules via Validated: a large-working-set PostgreSQL schema build whose index phase previously aborted here now completes and survives reboots. |
1 participant
callout_reset_on()dereferences its callout pointer immediately. Under concurrency the FreeBSD ARP/link-layer path can reachcallout_reset()on a link-layer entry (la->la_timer) that has raced with its own teardown, passing a null callout. The dereference then page-faults at address 0 deep in the network output path (arpresolve->ether_output->tcp_output), aborting the instance instead of harmlessly doing nothing.Guard the entry: if the callout pointer is null there is nothing to arm, so return 0 (no pending callout deactivated), matching the contract that resetting an already-inactive callout is a no-op.
Observed under a high-connection-count network workload; the abort is intermittent and race-dependent. Standalone change, no dependencies.