Data race on m_rspConnector lifetime across Gdb/Corellium/Esreven adapters

[xusheng6]

Summary

After #1073 landed a null check on CorelliumAdapter::BreakInto, that crash window is narrower but the underlying defect is still present and shared by every adapter that uses RspConnector. The null check makes the crash less likely, but it does not stop it from happening.

Why the null check is insufficient

DebuggerController::ExecuteAdapterAndWait deliberately uses two different mutexes (debuggercontroller.cpp:2678-2695). Pause/Quit/Detach take m_adapterMutex2 while everything else takes m_adapterMutex, with a comment that pause must not try to lock the running-operation mutex because it is held by the thread that is currently blocked inside the adapter. In other words BreakInto is designed to run concurrently with Go / ResponseHandler on a different thread.

Meanwhile, each adapter's ResponseHandler deletes and nulls its own m_rspConnector on target-exit (or comparable teardown), with no synchronization on the pointer field. For example, in CorelliumAdapter::ResponseHandler at corelliumadapter.cpp:861-864:

if (m_rspConnector)
{
    delete m_rspConnector;
    m_rspConnector = nullptr;
}

So BreakInto reads m_rspConnector, and ResponseHandler mutates it, with nothing in between. Mutating a field on one thread and reading it from another thread without correct synchronization is a data race. The null check added in #1073 only reduces the window between the load and the call to SendRaw; the connector can still be freed in between.

The EXCEPTION_ACCESS_VIOLATION_READ / 0x50 signature reported in BINARYNINJA-3X is consistent both with a true null deref (offset 0x50 inside RspConnector) and with a use-after-free where the freed block was zeroed. The post-#1073 occurrences are most likely the latter.

Affected adapters

The same raw RspConnector* pointer with the same teardown pattern exists in:

• core/adapters/corelliumadapter.cpp (h:50 — RspConnector* m_rspConnector{};)
• core/adapters/gdbadapter.cpp
• core/adapters/esrevenadapter.cpp

All three delete the connector inside their own ResponseHandler (or comparable teardown) while methods like BreakInto, WriteRegister, ReadMemory, etc. can read it from another thread. Anything that touches m_rspConnector outside the operation thread is exposed to the same race.

Suggested fix

Either:

1. Mutex: add a dedicated std::mutex per adapter that protects every read and write of m_rspConnector. BreakInto would acquire it around the SendRaw call; the teardown sites would acquire it around delete + null.
2. std::atomic<std::shared_ptr<RspConnector>>: change the field type, do lock-free atomic loads in callers, atomic-store an empty shared_ptr at teardown. Cleaner but touches every read site.

Either approach should be applied consistently to all three adapters. The shared_ptr form is friendlier if more call sites need protection; the mutex form is a smaller diff if only the teardown paths matter.

Related

• Crash inside BinaryNinjaDebugger::RspConnector::SendRaw #1066 (Sentry BINARYNINJA-3X) — closed by Fix crash in CorelliumAdapter::BreakInto when m_rspConnector is null #1073, but the same crash signature can recur until this is fixed.