Audit report AR-76

Reported privately in an independent security review, 2026-09-29.

Report date

2026-09-29

Kind

Bug

Status on master

Present

Proposed issue state

Open

Linked bugs

Bug 37: Double free when a thread exits while its connection is being destroyed

Summary

If a thread that made IPC calls exits at the moment its connection is destroyed on the event loop thread, both sides can tear down the same thread proxy and crash the process. The report is accurate, and it is a known open bug: Double free when a thread exits while its connection is being destroyed, with a fix pending in mp#361. The report's fix takes the same approach and adds a larger change to how cleanup callbacks are registered, which the bug does not need.

The report

Each thread that makes IPC calls keeps a map of thread proxies (ProxyClient<Thread>), one per connection. Cleanup of each entry should have one owner, but the exiting caller thread and the connection's disconnect callback can both tear down the same proxy and its callback. The report also says the cleanup list was tracked with iterators that could become invalid while the connection was running its cleanup callbacks, and that the thread proxy's destructor could read a connection pointer that another thread was clearing. A test that makes a call from a thread and lets it exit while another thread disconnects the client segfaulted in an ordinary release build of the libmultiprocess test program.

The report rates it low and asks for a decision before release. The trigger needs access to an enabled local IPC socket and timing that overlaps a thread's exit with a disconnect. A full Bitcoin Core node crash was not reproduced, and no peer-to-peer or RPC trigger was shown.

Proposed change

• When the connection's cleanup callback removes a thread proxy, it looks the entry up by connection and takes it out of the map while holding the thread's Waiter lock, then destroys it after releasing the lock. A new ThreadContext destructor, run when the thread exits, does the same with all its entries, so whichever side removes an entry first destroys it.
• Cleanup registrations become shared tokens instead of list iterators. A token records whether its callback has already been run or removed, so removing it twice, or after the connection has started running callbacks, is safe from any thread.
• The thread proxy destructor removes its token directly instead of waiting for the event loop thread.

The test "Caller thread exit during disconnect keeps thread client alive" repeats the exit-during-disconnect sequence 20 times. The report considered locking only the connection pointer read and rejected it because it would leave the double ownership and the invalid iterator in place.

Assessment

Accurate, and the same bug as an existing record. On libmultiprocess master, ThreadContext has no destructor, so a thread's maps are destroyed without a lock when it exits, while the connection's cleanup callback erases the same entry on the event loop thread through an iterator it saved earlier. That is the race recorded on the bug page, which was found while testing another change in August 2026 and reproduced with AddressSanitizer as a use-after-free followed by a double free.

The fix in mp#361, open since 2026-09-11, takes the same approach as the report: an explicit ThreadContext destructor that takes the maps under the Waiter lock, a cleanup callback that looks the entry up by connection and removes it under that lock, and destruction after the lock is released. It handles the invalid-iterator case without tokens: the thread proxy destructor skips removing its callback when its connection pointer is already null, which means the connection has already run and freed its cleanup list. Its test interleaves the two sides with a timing hook and fails on every run without the fix; the report's 20-round loop depends on timing.

The token change is more than this bug needs. It lets the thread proxy destructor take the event loop's mutex from the exiting thread instead of going through the event loop thread, which changes the locking rules in an area with a history of lock-order mistakes. The report says a libmultiprocess version of its change was prepared separately; mp#361 already covers it, so review there would help most. The release-build segfault was not repeated for this assessment.

Status

Present on master; fix pending in mp#361.

Reported privately in an independent security review on 2026-09-29.