From 73e6517d15b669a9ddfa69f4715416b06cf27387 Mon Sep 17 00:00:00 2001 From: Xusheng Date: Thu, 14 May 2026 11:25:44 -0400 Subject: [PATCH] Fix data race on RspConnector lifetime in Gdb/Corellium/Esreven adapters DebuggerController::ExecuteAdapterAndWait uses two separate mutexes so that Pause/Quit/Detach can run concurrently with a blocked Go/Receive on another thread (debuggercontroller.cpp:2678-2695). That design is intentional, but it means BreakInto races against ResponseHandler's own teardown of m_rspConnector on target exit. The previous raw-pointer field had no synchronization on either the load or the delete, so the read side could deref a freed object. Wrap the field in a small AtomicRspConnector helper (mutex-guarded shared_ptr) and have every accessor load a strong reference before use. Teardown sites now just store(nullptr) - the connector is destroyed when the last strong reference drops, never under an in-flight call. The same fix is applied to GdbAdapter and EsrevenAdapter since they share the pattern. Fixes #1074 Refs #1066 Co-Authored-By: Claude Opus 4.7 (1M context) --- core/adapters/corelliumadapter.cpp | 158 ++++++++++++++-------- core/adapters/corelliumadapter.h | 2 +- core/adapters/esrevenadapter.cpp | 207 ++++++++++++++++------------- core/adapters/esrevenadapter.h | 2 +- core/adapters/gdbadapter.cpp | 155 +++++++++++---------- core/adapters/gdbadapter.h | 2 +- core/adapters/rspconnector.h | 25 ++++ 7 files changed, 332 insertions(+), 219 deletions(-) diff --git a/core/adapters/corelliumadapter.cpp b/core/adapters/corelliumadapter.cpp index 02815426..408db0f5 100644 --- a/core/adapters/corelliumadapter.cpp +++ b/core/adapters/corelliumadapter.cpp @@ -86,7 +86,11 @@ bool CorelliumAdapter::LoadRegisterInfo() if (m_isTargetRunning) return false; - const auto xml = this->m_rspConnector->GetXml("target.xml"); + auto connector = m_rspConnector.load(); + if (!connector) + return false; + + const auto xml = connector->GetXml("target.xml"); pugi::xml_document doc{}; const auto parse_result = doc.load_string(xml.c_str()); @@ -226,9 +230,10 @@ bool CorelliumAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - this->m_rspConnector = new RspConnector(this->m_socket); - this->m_rspConnector->TransmitAndReceive(RspData("Hg0")); - this->m_rspConnector->NegotiateCapabilities( + auto connector = std::make_shared(this->m_socket); + m_rspConnector.store(connector); + connector->TransmitAndReceive(RspData("Hg0")); + connector->NegotiateCapabilities( { "swbreak+", "hwbreak+", "qRelocInsn+", "fork-events+", "vfork-events+", "exec-events+", "vContSupported+", "QThreadEvents+", "no-resumed+", "xmlRegisters=i386" } ); if ( !this->LoadRegisterInfo() ) @@ -242,7 +247,7 @@ bool CorelliumAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("?")); + const auto reply = connector->TransmitAndReceive(RspData("?")); auto map = RspConnector::PacketToUnorderedMap(reply); this->m_lastActiveThreadId = (uint32_t)map["thread"]; this->m_processPid = (uint32_t)map["thread"]; @@ -282,16 +287,14 @@ bool CorelliumAdapter::Connect(const std::string& server, std::uint32_t port) bool CorelliumAdapter::Detach() { - this->m_rspConnector->SendPayload(RspData("D")); + auto connector = m_rspConnector.load(); + if (connector) + connector->SendPayload(RspData("D")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); DebuggerEvent dbgevt; dbgevt.type = TargetExitedEventType; @@ -306,16 +309,14 @@ bool CorelliumAdapter::Quit() // Modern gdbserver uses vkill to kill the taget: // $vKill;7c3d#6e // $OK#9a - this->m_rspConnector->SendPayload(RspData("k")); + auto connector = m_rspConnector.load(); + if (connector) + connector->SendPayload(RspData("k")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); // TODO: we should only treat the target as exited when either 1) the remote side closes the socket, or, 2) the // remote side returns OK to the vkill request. @@ -335,9 +336,13 @@ std::vector CorelliumAdapter::GetThreadList() if (m_isTargetRunning) return {}; + auto connector = m_rspConnector.load(); + if (!connector) + return {}; + std::vector threads{}; - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("qfThreadInfo")); + auto reply = connector->TransmitAndReceive(RspData("qfThreadInfo")); while(reply.m_data[0] != 'l') { if (reply.m_data[0] != 'm') { LogWarn("thread list failed"); @@ -350,7 +355,7 @@ std::vector CorelliumAdapter::GetThreadList() for ( const auto& tid : tids ) threads.emplace_back(std::stoi(tid, nullptr, 16)); - reply = this->m_rspConnector->TransmitAndReceive(RspData("qsThreadInfo")); + reply = connector->TransmitAndReceive(RspData("qsThreadInfo")); } return threads; @@ -380,19 +385,23 @@ bool CorelliumAdapter::SetActiveThreadId(std::uint32_t tid) if (m_isTargetRunning) return false; - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) + auto connector = m_rspConnector.load(); + if (!connector) + return false; + + if ( connector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) { LogWarn("thread does not exist"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; @@ -408,6 +417,10 @@ DebugBreakpoint CorelliumAdapter::AddBreakpoint(const std::uintptr_t address, un if (m_isTargetRunning) return {}; + auto connector = m_rspConnector.load(); + if (!connector) + return {}; + if ( std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), DebugBreakpoint(address)) != this->m_debugBreakpoints.end()) return {}; @@ -419,7 +432,7 @@ DebugBreakpoint CorelliumAdapter::AddBreakpoint(const std::uintptr_t address, un // TODO: other archs have other values for kind, e.g., thumb2 needs a value of 2 or 3 here. // https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html - if (this->m_rspConnector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) return DebugBreakpoint{}; const auto new_breakpoint = DebugBreakpoint(address, this->m_internalBreakpointId++, true); @@ -433,6 +446,10 @@ bool CorelliumAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) if (m_isTargetRunning) return false; + auto connector = m_rspConnector.load(); + if (!connector) + return false; + if (auto location = std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), breakpoint); location == this->m_debugBreakpoints.end()) { return false; @@ -443,7 +460,7 @@ bool CorelliumAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) if (m_remoteArch == "aarch64") kind = 4; - if (this->m_rspConnector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) { LogWarn("rsp reply failure on remove breakpoint"); return false; @@ -534,6 +551,10 @@ std::unordered_map CorelliumAdapter::ReadAllRegister return {}; } + auto connector = m_rspConnector.load(); + if (!connector) + return {}; + // Sort the registers according to their index, as the g reply packet will provide values in the same order std::vector register_info_vec{}; for ( const auto& [register_name, register_info] : this->m_registerInfo ) @@ -545,7 +566,7 @@ std::unordered_map CorelliumAdapter::ReadAllRegister }); char request{'g'}; - const auto register_info_reply = this->m_rspConnector->TransmitAndReceive(RspData(&request, sizeof(request))); + const auto register_info_reply = connector->TransmitAndReceive(RspData(&request, sizeof(request))); auto register_info_reply_string = register_info_reply.AsString(); if ( register_info_reply_string.empty() ) { @@ -623,15 +644,19 @@ bool CorelliumAdapter::WriteRegister(const std::string& reg, intx::uint512 value if (!this->m_registerInfo.contains(reg)) return false; + auto connector = m_rspConnector.load(); + if (!connector) + return false; + const auto newRegString = m_isBigEndian ? uint512ToBigEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8) : uint512ToLittleEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8); - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("P{:02X}={}", + const auto reply = connector->TransmitAndReceive(RspData("P{:02X}={}", this->m_registerInfo[reg].m_regNum, newRegString)); if (reply.m_data[0]) return true; char query{'g'}; - const auto generic_query = this->m_rspConnector->TransmitAndReceive(RspData(&query, sizeof(query))); + const auto generic_query = connector->TransmitAndReceive(RspData(&query, sizeof(query))); const auto register_offset = this->m_registerInfo[reg].m_offset; // TODO: check if this works for aarch64 @@ -639,7 +664,7 @@ bool CorelliumAdapter::WriteRegister(const std::string& reg, intx::uint512 value const auto second_half = generic_query.AsString().substr(2 * ((register_offset + this->m_registerInfo[reg].m_bitSize) / 8) ); const auto payload = "G" + first_half + newRegString + second_half; - if ( this->m_rspConnector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) + if ( connector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) return false; // TODO: we do not need to invalidate all register caches, we could probably just update the necessary ones here @@ -653,7 +678,11 @@ DataBuffer CorelliumAdapter::ReadMemory(std::uintptr_t address, std::size_t size if (m_isTargetRunning) return DataBuffer{}; - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); + auto connector = m_rspConnector.load(); + if (!connector) + return DataBuffer{}; + + auto reply = connector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); if (reply.m_data[0] == 'E') return DataBuffer{}; @@ -695,6 +724,10 @@ bool CorelliumAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buf if (m_isTargetRunning) return false; + auto connector = m_rspConnector.load(); + if (!connector) + return false; + size_t size = buffer.GetLength(); DataBuffer dest(2 * size); @@ -706,7 +739,7 @@ bool CorelliumAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buf dest[2 * index + 1] = hex[1]; } - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); + auto reply = connector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); if (reply.AsString() != "OK") return false; @@ -719,9 +752,13 @@ std::string CorelliumAdapter::GetRemoteFile(const std::string& path) if (m_isTargetRunning) return ""; + auto connector = m_rspConnector.load(); + if (!connector) + return ""; + RspData output; int32_t error; - int32_t ret = this->m_rspConnector->HostFileIO(RspData("vFile:setfs:0"), output, error); + int32_t ret = connector->HostFileIO(RspData("vFile:setfs:0"), output, error); if (ret < 0) { LogWarn("Could not set remote filesystem"); @@ -732,7 +769,7 @@ std::string CorelliumAdapter::GetRemoteFile(const std::string& path) for ( const auto& ch : path ) path_hex_string += fmt::format("{:02X}", ch); - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:open:{},{:X},{:X}", path_hex_string.c_str(), 0, 0), output, error); if (ret < 0) { @@ -748,7 +785,7 @@ std::string CorelliumAdapter::GetRemoteFile(const std::string& path) while(true) { - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:pread:{:X},{:X},{:X}", fd, blockSize, offset), output, error); if (ret < 0) { @@ -764,12 +801,12 @@ std::string CorelliumAdapter::GetRemoteFile(const std::string& path) ret, output.AsString().length()); return data; } - + data += output.AsString(); offset += output.AsString().length(); } - ret = this->m_rspConnector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); + ret = connector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); if (ret) LogWarn("host i/o close() failed, result=%d, errno=%d", ret, error); @@ -789,11 +826,12 @@ std::string CorelliumAdapter::GetTargetArchitecture() bool CorelliumAdapter::BreakInto() { - if (!m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (!m_isTargetRunning || !connector) return false; char var = '\x03'; - this->m_rspConnector->SendRaw(RspData(&var, sizeof(var))); + connector->SendRaw(RspData(&var, sizeof(var))); m_isTargetRunning = false; return true; } @@ -801,9 +839,13 @@ bool CorelliumAdapter::BreakInto() DebugStopReason CorelliumAdapter::ResponseHandler() { + auto connector = m_rspConnector.load(); + if (!connector) + return DebugStopReason::UnknownReason; + while (true) { - const RspData reply = m_rspConnector->ReceiveRspData(); + const RspData reply = connector->ReceiveRspData(); if (reply[0] == 'T') { // Target stopped @@ -858,11 +900,7 @@ DebugStopReason CorelliumAdapter::ResponseHandler() this->m_socket->Kill(); m_isTargetRunning = false; - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); return DebugStopReason::ProcessExited; break; @@ -918,10 +956,14 @@ DebugStopReason CorelliumAdapter::ResponseHandler() // this should return the information about the target stop DebugStopReason CorelliumAdapter::GenericGo(const std::string& goCommand) { + auto connector = m_rspConnector.load(); + if (!connector) + return DebugStopReason::UnknownReason; + m_isTargetRunning = true; // TODO: these two calls should be combined - m_rspConnector->SendPayload(RspData(goCommand)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(goCommand)); + connector->ExpectAck(); return ResponseHandler(); } @@ -971,7 +1013,11 @@ std::string CorelliumAdapter::InvokeBackendCommand(const std::string& command) else if (command.substr(0, 8) == "monitor ") return RunMonitorCommand(command.substr(4)); - auto reply = this->m_rspConnector->TransmitAndReceive(RspData(command)); + auto connector = m_rspConnector.load(); + if (!connector) + return ""; + + auto reply = connector->TransmitAndReceive(RspData(command)); return reply.AsString(); } @@ -1005,6 +1051,10 @@ static std::string HexToAscii(const std::string& hex) std::string CorelliumAdapter::RunMonitorCommand(const std::string& command) { + auto connector = m_rspConnector.load(); + if (!connector) + return ""; + std::string commandToSend = "qRcmd,"; for (const auto& c: command) { @@ -1012,13 +1062,13 @@ std::string CorelliumAdapter::RunMonitorCommand(const std::string& command) commandToSend += ("0123456789abcdef"[c & 0x0F]); } - m_rspConnector->SendPayload(RspData(commandToSend)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(commandToSend)); + connector->ExpectAck(); std::string result; while (true) { - auto replyChunk = this->m_rspConnector->ReceiveRspData(); + auto replyChunk = connector->ReceiveRspData(); if (replyChunk.AsString() == "OK" || replyChunk.AsString().empty()) break; @@ -1073,7 +1123,8 @@ bool CorelliumAdapter::SupportFeature(DebugAdapterCapacity feature) bool CorelliumAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; std::string command; @@ -1099,13 +1150,14 @@ bool CorelliumAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointTy return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } bool CorelliumAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; std::string command; @@ -1131,7 +1183,7 @@ bool CorelliumAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpoin return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } diff --git a/core/adapters/corelliumadapter.h b/core/adapters/corelliumadapter.h index 49b6342d..f5b784dc 100644 --- a/core/adapters/corelliumadapter.h +++ b/core/adapters/corelliumadapter.h @@ -47,7 +47,7 @@ namespace BinaryNinjaDebugger using register_pair = std::pair; Socket* m_socket; - RspConnector* m_rspConnector{}; + AtomicRspConnector m_rspConnector; std::map m_registerInfo{}; std::optional> m_regCache; diff --git a/core/adapters/esrevenadapter.cpp b/core/adapters/esrevenadapter.cpp index c55084e8..3b0302cb 100644 --- a/core/adapters/esrevenadapter.cpp +++ b/core/adapters/esrevenadapter.cpp @@ -79,7 +79,8 @@ bool EsrevenAdapter::ExecuteWithArgs(const std::string &path, const std::string bool EsrevenAdapter::Attach(std::uint32_t pid) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) { LogWarn("EsrevenAdapter::Attach called without an active connection. " "Please connect to the debug server first."); @@ -87,7 +88,7 @@ bool EsrevenAdapter::Attach(std::uint32_t pid) } // Send rvn:select-process to activate process filtering at runtime - auto response = m_rspConnector->TransmitAndReceive( + auto response = connector->TransmitAndReceive( RspData(fmt::format("rvn:select-process:{}", pid))); std::string responseStr = response.AsString(); @@ -108,7 +109,7 @@ bool EsrevenAdapter::Attach(std::uint32_t pid) ClearCachedBreakpoints(); // Query the initial stop reason after selecting the process - const auto reply = m_rspConnector->TransmitAndReceive(RspData("?")); + const auto reply = connector->TransmitAndReceive(RspData("?")); auto map = RspConnector::PacketToUnorderedMap(reply); m_lastActiveThreadId = (uint32_t)map["thread"]; m_isTargetRunning = false; @@ -134,10 +135,11 @@ bool EsrevenAdapter::Attach(std::uint32_t pid) bool EsrevenAdapter::LoadRegisterInfo() { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; - const auto xml = this->m_rspConnector->GetXml("target.xml"); + const auto xml = connector->GetXml("target.xml"); pugi::xml_document doc{}; const auto parse_result = doc.load_string(xml.c_str()); @@ -193,7 +195,7 @@ bool EsrevenAdapter::LoadRegisterInfo() else if (node.name() == "xi:include"s ) { auto includePath = node.attribute("href").value(); - const auto includedXml = this->m_rspConnector->GetXml(includePath); + const auto includedXml = connector->GetXml(includePath); if (includedXml.empty()) continue; @@ -301,13 +303,14 @@ bool EsrevenAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - this->m_rspConnector = new RspConnector(this->m_socket); - this->m_rspConnector->TransmitAndReceive(RspData("Hg0")); - this->m_rspConnector->NegotiateCapabilities( + auto connector = std::make_shared(this->m_socket); + m_rspConnector.store(connector); + connector->TransmitAndReceive(RspData("Hg0")); + connector->NegotiateCapabilities( { "swbreak+", "hwbreak+", "qRelocInsn+", "fork-events+", "vfork-events+", "exec-events+", "vContSupported+", "QThreadEvents+", "no-resumed+", "xmlRegisters=i386" } ); - auto capacities = m_rspConnector->GetServerCapabilities(); + auto capacities = connector->GetServerCapabilities(); if (std::find(capacities.begin(), capacities.end(), "ReverseContinue") != capacities.end()) m_canReverseContinue = true; if (std::find(capacities.begin(), capacities.end(), "ReverseStep") != capacities.end()) @@ -324,7 +327,7 @@ bool EsrevenAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("?")); + const auto reply = connector->TransmitAndReceive(RspData("?")); auto map = RspConnector::PacketToUnorderedMap(reply); this->m_lastActiveThreadId = (uint32_t)map["thread"]; this->m_processPid = (uint32_t)map["thread"]; @@ -388,13 +391,14 @@ bool EsrevenAdapter::ConnectToDebugServer(const std::string &server, std::uint32 return false; } - this->m_rspConnector = new RspConnector(this->m_socket); - this->m_rspConnector->TransmitAndReceive(RspData("Hg0")); - this->m_rspConnector->NegotiateCapabilities( + auto connector = std::make_shared(this->m_socket); + m_rspConnector.store(connector); + connector->TransmitAndReceive(RspData("Hg0")); + connector->NegotiateCapabilities( { "swbreak+", "hwbreak+", "qRelocInsn+", "fork-events+", "vfork-events+", "exec-events+", "vContSupported+", "QThreadEvents+", "no-resumed+", "xmlRegisters=i386" }); - auto capacities = m_rspConnector->GetServerCapabilities(); + auto capacities = connector->GetServerCapabilities(); if (std::find(capacities.begin(), capacities.end(), "ReverseContinue") != capacities.end()) m_canReverseContinue = true; if (std::find(capacities.begin(), capacities.end(), "ReverseStep") != capacities.end()) @@ -417,37 +421,34 @@ bool EsrevenAdapter::ConnectToDebugServer(const std::string &server, std::uint32 bool EsrevenAdapter::DisconnectDebugServer() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return true; - this->m_rspConnector->SendPayload(RspData("D")); + connector->SendPayload(RspData("D")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); ClearCachedBreakpoints(); - delete m_rspConnector; - m_rspConnector = nullptr; + m_rspConnector.store(nullptr); return true; } bool EsrevenAdapter::Detach() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return false; - this->m_rspConnector->SendPayload(RspData("D")); + connector->SendPayload(RspData("D")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); ClearCachedBreakpoints(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); DebuggerEvent dbgevt; dbgevt.type = TargetExitedEventType; @@ -459,23 +460,20 @@ bool EsrevenAdapter::Detach() bool EsrevenAdapter::Quit() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return false; // Modern gdbserver uses vkill to kill the taget: // $vKill;7c3d#6e // $OK#9a - this->m_rspConnector->SendPayload(RspData("k")); + connector->SendPayload(RspData("k")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); ClearCachedBreakpoints(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); // TODO: we should only treat the target as exited when either 1) the remote side closes the socket, or, 2) the // remote side returns OK to the vkill request. @@ -503,11 +501,12 @@ std::vector EsrevenAdapter::GetThreadList() return threads; } - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; // Use the custom rvn:list-threads packet - auto response = m_rspConnector->TransmitAndReceive(RspData("rvn:list-threads")); + auto response = connector->TransmitAndReceive(RspData("rvn:list-threads")); std::string jsonStr = response.AsString(); // Check if we got a valid JSON response @@ -809,22 +808,23 @@ bool EsrevenAdapter::SetActiveThread(const DebugThread& thread) bool EsrevenAdapter::SetActiveThreadId(std::uint32_t tid) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) + if ( connector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) { LogWarn("thread does not exist"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; @@ -837,7 +837,8 @@ bool EsrevenAdapter::SetActiveThreadId(std::uint32_t tid) DebugBreakpoint EsrevenAdapter::AddBreakpoint(const std::uintptr_t address, unsigned long breakpoint_type) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; if ( std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), @@ -851,7 +852,7 @@ DebugBreakpoint EsrevenAdapter::AddBreakpoint(const std::uintptr_t address, unsi // TODO: other archs have other values for kind, e.g., thumb2 needs a value of 2 or 3 here. // https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html - if (this->m_rspConnector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) return DebugBreakpoint{}; const auto new_breakpoint = DebugBreakpoint(address, this->m_internalBreakpointId++, true, SoftwareBreakpoint); @@ -862,7 +863,8 @@ DebugBreakpoint EsrevenAdapter::AddBreakpoint(const std::uintptr_t address, unsi bool EsrevenAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; if (auto location = std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), breakpoint); @@ -875,7 +877,7 @@ bool EsrevenAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) if (m_remoteArch == "aarch64") kind = 4; - if (this->m_rspConnector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) { LogDebug("rsp reply failure on remove breakpoint"); return false; @@ -957,7 +959,8 @@ static intx::uint512 parseBigEndianHexToUint512(const std::string& hex) { std::unordered_map EsrevenAdapter::ReadAllRegisters() { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; if (m_regCache.has_value()) @@ -980,7 +983,7 @@ std::unordered_map EsrevenAdapter::ReadAllRegisters( }); char request{'g'}; - const auto register_info_reply = this->m_rspConnector->TransmitAndReceive(RspData(&request, sizeof(request))); + const auto register_info_reply = connector->TransmitAndReceive(RspData(&request, sizeof(request))); auto register_info_reply_string = register_info_reply.AsString(); if ( register_info_reply_string.empty() ) { @@ -1053,7 +1056,8 @@ static std::string uint512ToBigEndianHex(const intx::uint512& value, size_t widt bool EsrevenAdapter::WriteRegister(const std::string& reg, intx::uint512 value) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; if (!this->m_registerInfo.contains(reg)) @@ -1061,14 +1065,14 @@ bool EsrevenAdapter::WriteRegister(const std::string& reg, intx::uint512 value) const auto newRegString = m_isBigEndian ? uint512ToBigEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8) : uint512ToLittleEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8); - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("P{:02X}={}", + const auto reply = connector->TransmitAndReceive(RspData("P{:02X}={}", this->m_registerInfo[reg].m_regNum, newRegString)); if (reply.m_data[0]) return true; char query{'g'}; - const auto generic_query = this->m_rspConnector->TransmitAndReceive(RspData(&query, sizeof(query))); + const auto generic_query = connector->TransmitAndReceive(RspData(&query, sizeof(query))); const auto register_offset = this->m_registerInfo[reg].m_offset; // TODO: check if this works for aarch64 @@ -1076,7 +1080,7 @@ bool EsrevenAdapter::WriteRegister(const std::string& reg, intx::uint512 value) const auto second_half = generic_query.AsString().substr(2 * ((register_offset + this->m_registerInfo[reg].m_bitSize) / 8) ); const auto payload = "G" + first_half + newRegString + second_half; - if ( this->m_rspConnector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) + if ( connector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) return false; // TODO: we do not need to invalidate all register caches, we could probably just update the necessary ones here @@ -1087,10 +1091,11 @@ bool EsrevenAdapter::WriteRegister(const std::string& reg, intx::uint512 value) DataBuffer EsrevenAdapter::ReadMemory(std::uintptr_t address, std::size_t size) { // This means whether the target is running. If it is, then we cannot read memory at the moment - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return DataBuffer{}; - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); + auto reply = connector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); if (reply.m_data[0] == 'E') return DataBuffer{}; @@ -1132,6 +1137,10 @@ bool EsrevenAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffe if (m_isTargetRunning) return false; + auto connector = m_rspConnector.load(); + if (!connector) + return false; + size_t size = buffer.GetLength(); DataBuffer dest(2 * size); @@ -1143,7 +1152,7 @@ bool EsrevenAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffe dest[2 * index + 1] = hex[1]; } - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); + auto reply = connector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); if (reply.AsString() != "OK") return false; @@ -1153,12 +1162,13 @@ bool EsrevenAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffe std::string EsrevenAdapter::GetRemoteFile(const std::string& path) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return ""; RspData output; int32_t error; - int32_t ret = this->m_rspConnector->HostFileIO(RspData("vFile:setfs:0"), output, error); + int32_t ret = connector->HostFileIO(RspData("vFile:setfs:0"), output, error); if (ret < 0) { LogDebug("Could not set remote filesystem"); @@ -1169,7 +1179,7 @@ std::string EsrevenAdapter::GetRemoteFile(const std::string& path) for ( const auto& ch : path ) path_hex_string += fmt::format("{:02X}", ch); - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:open:{},{:X},{:X}", path_hex_string.c_str(), 0, 0), output, error); if (ret < 0) { @@ -1185,7 +1195,7 @@ std::string EsrevenAdapter::GetRemoteFile(const std::string& path) while(true) { - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:pread:{:X},{:X},{:X}", fd, blockSize, offset), output, error); if (ret < 0) { @@ -1209,7 +1219,7 @@ std::string EsrevenAdapter::GetRemoteFile(const std::string& path) offset += output.AsString().length(); } - ret = this->m_rspConnector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); + ret = connector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); if (ret) { auto msg = fmt::format("host i/o close() failed, result={}, errno={}", ret, error); @@ -1228,12 +1238,13 @@ std::vector EsrevenAdapter::GetModuleList() if (m_isTargetRunning) return {}; - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; // Use the custom reven list-current-mappings packet // Request all mappings (process, kernel, etc.) - auto response = m_rspConnector->TransmitAndReceive(RspData("rvn:list-current-mappings:all")); + auto response = connector->TransmitAndReceive(RspData("rvn:list-current-mappings:all")); std::string jsonStr = response.AsString(); // Check if we got a valid JSON response @@ -1443,7 +1454,8 @@ std::vector EsrevenAdapter::GetTTDMemoryAccessForAddress(uint64_ if (m_isTargetRunning) return {}; - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; // Convert TTDMemoryAccessType flags to comma-separated string @@ -1467,7 +1479,7 @@ std::vector EsrevenAdapter::GetTTDMemoryAccessForAddress(uint64_ } // Send the custom REVEN packet: rvn:get-memory-accesses::: - auto response = m_rspConnector->TransmitAndReceive( + auto response = connector->TransmitAndReceive( RspData("rvn:get-memory-accesses:{:x}:{:x}:{}", startAddress, endAddress, typesStr)); std::string jsonStr = response.AsString(); @@ -1593,7 +1605,8 @@ std::vector EsrevenAdapter::GetTTDMemoryAcce if (m_isTargetRunning) return {}; - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; // Convert TTDMemoryAccessType flags to comma-separated string @@ -1621,7 +1634,7 @@ std::vector EsrevenAdapter::GetTTDMemoryAcce uint64_t endTransition = endTime.sequence; // Send the custom REVEN packet with time range: rvn:get-memory-accesses::::: - auto response = m_rspConnector->TransmitAndReceive( + auto response = connector->TransmitAndReceive( RspData("rvn:get-memory-accesses:{:x}:{:x}:{}:{}:{}", startAddress, endAddress, typesStr, startTransition, endTransition)); std::string jsonStr = response.AsString(); @@ -1749,11 +1762,12 @@ std::string EsrevenAdapter::GetTargetArchitecture() bool EsrevenAdapter::BreakInto() { - if (!m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (!m_isTargetRunning || !connector) return false; char var = '\x03'; - this->m_rspConnector->SendRaw(RspData(&var, sizeof(var))); + connector->SendRaw(RspData(&var, sizeof(var))); m_isTargetRunning = false; return true; } @@ -1761,12 +1775,13 @@ bool EsrevenAdapter::BreakInto() DebugStopReason EsrevenAdapter::ResponseHandler(bool notifyStopped) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return InternalError; while (true) { - const RspData reply = m_rspConnector->ReceiveRspData(); + const RspData reply = connector->ReceiveRspData(); if (reply[0] == 'T') { // Target stopped @@ -1834,11 +1849,7 @@ DebugStopReason EsrevenAdapter::ResponseHandler(bool notifyStopped) this->m_socket->Kill(); m_isTargetRunning = false; - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); return DebugStopReason::ProcessExited; break; @@ -1897,13 +1908,14 @@ DebugStopReason EsrevenAdapter::ResponseHandler(bool notifyStopped) // this should return the information about the target stop DebugStopReason EsrevenAdapter::GenericGo(const std::string& goCommand, bool notifyStopped) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return InternalError; m_isTargetRunning = true; // TODO: these two calls should be combined - m_rspConnector->SendPayload(RspData(goCommand)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(goCommand)); + connector->ExpectAck(); return ResponseHandler(notifyStopped); } @@ -2003,7 +2015,8 @@ bool EsrevenAdapter::StepOverReverse() bool EsrevenAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) { // Cache the hardware breakpoint to be applied when target stops or connector becomes available PendingHardwareBreakpoint pending(address, type, size); @@ -2038,13 +2051,14 @@ bool EsrevenAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } bool EsrevenAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) { // Remove from pending list if target is running or connector not available PendingHardwareBreakpoint pending(address, type, size); @@ -2080,7 +2094,7 @@ bool EsrevenAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointT return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } @@ -2160,7 +2174,8 @@ bool EsrevenAdapter::StepReturnReverse() std::string EsrevenAdapter::InvokeBackendCommand(const std::string& command) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; if (command.substr(0, 4) == "mon ") @@ -2263,7 +2278,7 @@ std::string EsrevenAdapter::InvokeBackendCommand(const std::string& command) // Send the appropriate Z/z packet char zChar = isDelete ? 'z' : 'Z'; - auto reply = m_rspConnector->TransmitAndReceive(RspData("{}{},{:x},{}", + auto reply = connector->TransmitAndReceive(RspData("{}{},{:x},{}", zChar, bpType, address, kind)); if (reply.AsString() == "OK") @@ -2284,7 +2299,7 @@ std::string EsrevenAdapter::InvokeBackendCommand(const std::string& command) } } - auto reply = this->m_rspConnector->TransmitAndReceive(RspData(command)); + auto reply = connector->TransmitAndReceive(RspData(command)); return reply.AsString(); } @@ -2318,7 +2333,8 @@ static std::string HexToAscii(const std::string& hex) std::string EsrevenAdapter::RunMonitorCommand(const std::string& command) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; std::string commandToSend = "qRcmd,"; @@ -2328,13 +2344,13 @@ std::string EsrevenAdapter::RunMonitorCommand(const std::string& command) commandToSend += ("0123456789abcdef"[c & 0x0F]); } - m_rspConnector->SendPayload(RspData(commandToSend)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(commandToSend)); + connector->ExpectAck(); std::string result; while (true) { - auto replyChunk = this->m_rspConnector->ReceiveRspData(); + auto replyChunk = connector->ReceiveRspData(); if (replyChunk.AsString() == "OK" || replyChunk.AsString().empty()) break; @@ -2494,8 +2510,12 @@ std::vector EsrevenAdapter::GetProcessList() if (m_isTargetRunning) return {}; + auto connector = m_rspConnector.load(); + if (!connector) + return {}; + // Use the custom reven list-processes packet - auto response = m_rspConnector->TransmitAndReceive(RspData("rvn:list-processes")); + auto response = connector->TransmitAndReceive(RspData("rvn:list-processes")); std::string jsonStr = response.AsString(); // Check if we got a valid JSON response @@ -2856,10 +2876,11 @@ Ref EsrevenAdapterType::RegisterAdapterSettings() TTDPosition EsrevenAdapter::GetCurrentTTDPosition() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return TTDPosition(); - auto reply = m_rspConnector->TransmitAndReceive( + auto reply = connector->TransmitAndReceive( RspData("rvn:get-current-transition"), "ack_then_reply", nullptr, std::chrono::milliseconds(5000)); @@ -2902,7 +2923,7 @@ TTDPosition EsrevenAdapter::GetCurrentTTDPosition() bool EsrevenAdapter::SetTTDPosition(const TTDPosition& position) { - if (!m_rspConnector) + if (!m_rspConnector.load()) return false; DebuggerEvent dbgevt; @@ -2926,6 +2947,10 @@ std::vector EsrevenAdapter::GetTTDCallsForSymbols(const std::strin return events; } + auto connector = m_rspConnector.load(); + if (!connector) + return events; + // Get settings auto adapterSettings = GetAdapterSettings(); BNSettingsScope scope = SettingsResourceScope; @@ -2966,7 +2991,7 @@ std::vector EsrevenAdapter::GetTTDCallsForSymbols(const std::strin } // Send with custom timeout - auto reply = m_rspConnector->TransmitAndReceive( + auto reply = connector->TransmitAndReceive( RspData(packet), "ack_then_reply", nullptr, diff --git a/core/adapters/esrevenadapter.h b/core/adapters/esrevenadapter.h index a1c3689e..6d3c00f5 100644 --- a/core/adapters/esrevenadapter.h +++ b/core/adapters/esrevenadapter.h @@ -47,7 +47,7 @@ namespace BinaryNinjaDebugger using register_pair = std::pair; Socket* m_socket; - RspConnector* m_rspConnector{}; + AtomicRspConnector m_rspConnector; std::map m_registerInfo{}; std::optional> m_regCache; diff --git a/core/adapters/gdbadapter.cpp b/core/adapters/gdbadapter.cpp index e60e3619..ab638d77 100644 --- a/core/adapters/gdbadapter.cpp +++ b/core/adapters/gdbadapter.cpp @@ -85,10 +85,11 @@ bool GdbAdapter::Attach(std::uint32_t pid) bool GdbAdapter::LoadRegisterInfo() { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; - const auto xml = this->m_rspConnector->GetXml("target.xml"); + const auto xml = connector->GetXml("target.xml"); pugi::xml_document doc{}; const auto parse_result = doc.load_string(xml.c_str()); @@ -144,7 +145,7 @@ bool GdbAdapter::LoadRegisterInfo() else if (node.name() == "xi:include"s ) { auto includePath = node.attribute("href").value(); - const auto includedXml = this->m_rspConnector->GetXml(includePath); + const auto includedXml = connector->GetXml(includePath); if (includedXml.empty()) continue; @@ -252,13 +253,14 @@ bool GdbAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - this->m_rspConnector = new RspConnector(this->m_socket); - this->m_rspConnector->TransmitAndReceive(RspData("Hg0")); - this->m_rspConnector->NegotiateCapabilities( + auto connector = std::make_shared(this->m_socket); + m_rspConnector.store(connector); + connector->TransmitAndReceive(RspData("Hg0")); + connector->NegotiateCapabilities( { "swbreak+", "hwbreak+", "qRelocInsn+", "fork-events+", "vfork-events+", "exec-events+", "vContSupported+", "QThreadEvents+", "no-resumed+", "xmlRegisters=i386" } ); - auto capacities = m_rspConnector->GetServerCapabilities(); + auto capacities = connector->GetServerCapabilities(); if (std::find(capacities.begin(), capacities.end(), "ReverseContinue") != capacities.end()) m_canReverseContinue = true; if (std::find(capacities.begin(), capacities.end(), "ReverseStep") != capacities.end()) @@ -275,7 +277,7 @@ bool GdbAdapter::Connect(const std::string& server, std::uint32_t port) return false; } - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("?")); + const auto reply = connector->TransmitAndReceive(RspData("?")); auto map = RspConnector::PacketToUnorderedMap(reply); this->m_lastActiveThreadId = (uint32_t)map["thread"]; this->m_processPid = (uint32_t)map["thread"]; @@ -303,20 +305,17 @@ bool GdbAdapter::ConnectToDebugServer(const std::string &server, std::uint32_t p bool GdbAdapter::Detach() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return false; - this->m_rspConnector->SendPayload(RspData("D")); + connector->SendPayload(RspData("D")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); ClearCachedBreakpoints(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); DebuggerEvent dbgevt; dbgevt.type = TargetExitedEventType; @@ -328,23 +327,20 @@ bool GdbAdapter::Detach() bool GdbAdapter::Quit() { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return false; // Modern gdbserver uses vkill to kill the taget: // $vKill;7c3d#6e // $OK#9a - this->m_rspConnector->SendPayload(RspData("k")); + connector->SendPayload(RspData("k")); this->m_socket->Kill(); m_isTargetRunning = false; InvalidateCache(); ClearCachedBreakpoints(); - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); // TODO: we should only treat the target as exited when either 1) the remote side closes the socket, or, 2) the // remote side returns OK to the vkill request. @@ -361,12 +357,13 @@ bool GdbAdapter::Quit() std::vector GdbAdapter::GetThreadList() { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; std::vector threads{}; - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("qfThreadInfo")); + auto reply = connector->TransmitAndReceive(RspData("qfThreadInfo")); while(reply.m_data[0] != 'l') { if (reply.m_data[0] != 'm') { LogWarn("RSP thread list error"); @@ -379,7 +376,7 @@ std::vector GdbAdapter::GetThreadList() for ( const auto& tid : tids ) threads.emplace_back(std::stoi(tid, nullptr, 16)); - reply = this->m_rspConnector->TransmitAndReceive(RspData("qsThreadInfo")); + reply = connector->TransmitAndReceive(RspData("qsThreadInfo")); } return threads; @@ -406,22 +403,23 @@ bool GdbAdapter::SetActiveThread(const DebugThread& thread) bool GdbAdapter::SetActiveThreadId(std::uint32_t tid) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) + if ( connector->TransmitAndReceive(RspData(string("T{:x}"), tid)).AsString() != "OK" ) { LogWarn("thread does not exist"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hc{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; } - if ( this->m_rspConnector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") + if ( connector->TransmitAndReceive(RspData(string("Hg{:x}"), tid)).AsString() != "OK") { LogWarn("failed to set thread"); return false; @@ -434,7 +432,8 @@ bool GdbAdapter::SetActiveThreadId(std::uint32_t tid) DebugBreakpoint GdbAdapter::AddBreakpoint(const std::uintptr_t address, unsigned long breakpoint_type) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; if ( std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), @@ -448,7 +447,7 @@ DebugBreakpoint GdbAdapter::AddBreakpoint(const std::uintptr_t address, unsigned // TODO: other archs have other values for kind, e.g., thumb2 needs a value of 2 or 3 here. // https://sourceware.org/gdb/current/onlinedocs/gdb/ARM-Breakpoint-Kinds.html - if (this->m_rspConnector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("Z0,{:x},{}", address, kind)).AsString() != "OK" ) return DebugBreakpoint{}; const auto new_breakpoint = DebugBreakpoint(address, this->m_internalBreakpointId++, true, SoftwareBreakpoint); @@ -459,7 +458,8 @@ DebugBreakpoint GdbAdapter::AddBreakpoint(const std::uintptr_t address, unsigned bool GdbAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; if (auto location = std::find(this->m_debugBreakpoints.begin(), this->m_debugBreakpoints.end(), breakpoint); @@ -472,7 +472,7 @@ bool GdbAdapter::RemoveBreakpoint(const DebugBreakpoint& breakpoint) if (m_remoteArch == "aarch64") kind = 4; - if (this->m_rspConnector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) + if (connector->TransmitAndReceive(RspData("z0,{:x},{}", breakpoint.m_address, kind)).AsString() != "OK" ) { LogDebug("rsp reply failure on remove breakpoint"); return false; @@ -554,7 +554,8 @@ static intx::uint512 parseBigEndianHexToUint512(const std::string& hex) { std::unordered_map GdbAdapter::ReadAllRegisters() { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return {}; if (m_regCache.has_value()) @@ -577,7 +578,7 @@ std::unordered_map GdbAdapter::ReadAllRegisters() }); char request{'g'}; - const auto register_info_reply = this->m_rspConnector->TransmitAndReceive(RspData(&request, sizeof(request))); + const auto register_info_reply = connector->TransmitAndReceive(RspData(&request, sizeof(request))); auto register_info_reply_string = register_info_reply.AsString(); if ( register_info_reply_string.empty() ) { @@ -650,7 +651,8 @@ static std::string uint512ToBigEndianHex(const intx::uint512& value, size_t widt bool GdbAdapter::WriteRegister(const std::string& reg, intx::uint512 value) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return false; if (!this->m_registerInfo.contains(reg)) @@ -658,13 +660,13 @@ bool GdbAdapter::WriteRegister(const std::string& reg, intx::uint512 value) const auto newRegString = m_isBigEndian ? uint512ToBigEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8) : uint512ToLittleEndianHex(value, this->m_registerInfo[reg].m_bitSize / 8); - const auto reply = this->m_rspConnector->TransmitAndReceive(RspData("P{:02X}={}", + const auto reply = connector->TransmitAndReceive(RspData("P{:02X}={}", this->m_registerInfo[reg].m_regNum, newRegString)); if (reply.m_data[0]) return true; char query{'g'}; - const auto generic_query = this->m_rspConnector->TransmitAndReceive(RspData(&query, sizeof(query))); + const auto generic_query = connector->TransmitAndReceive(RspData(&query, sizeof(query))); const auto register_offset = this->m_registerInfo[reg].m_offset; // TODO: check if this works for aarch64 @@ -672,7 +674,7 @@ bool GdbAdapter::WriteRegister(const std::string& reg, intx::uint512 value) const auto second_half = generic_query.AsString().substr(2 * ((register_offset + this->m_registerInfo[reg].m_bitSize) / 8) ); const auto payload = "G" + first_half + newRegString + second_half; - if ( this->m_rspConnector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) + if ( connector->TransmitAndReceive(RspData(payload)).AsString() != "OK" ) return false; // TODO: we do not need to invalidate all register caches, we could probably just update the necessary ones here @@ -683,10 +685,11 @@ bool GdbAdapter::WriteRegister(const std::string& reg, intx::uint512 value) DataBuffer GdbAdapter::ReadMemory(std::uintptr_t address, std::size_t size) { // This means whether the target is running. If it is, then we cannot read memory at the moment - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return DataBuffer{}; - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); + auto reply = connector->TransmitAndReceive(RspData("m{:x},{:x}", address, size)); if (reply.m_data[0] == 'E') return DataBuffer{}; @@ -728,6 +731,10 @@ bool GdbAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffer) if (m_isTargetRunning) return false; + auto connector = m_rspConnector.load(); + if (!connector) + return false; + size_t size = buffer.GetLength(); DataBuffer dest(2 * size); @@ -739,7 +746,7 @@ bool GdbAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffer) dest[2 * index + 1] = hex[1]; } - auto reply = this->m_rspConnector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); + auto reply = connector->TransmitAndReceive(RspData("M{:x},{:x}:{}", address, size, dest.ToEscapedString())); if (reply.AsString() != "OK") return false; @@ -749,12 +756,13 @@ bool GdbAdapter::WriteMemory(std::uintptr_t address, const DataBuffer& buffer) std::string GdbAdapter::GetRemoteFile(const std::string& path) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) return ""; RspData output; int32_t error; - int32_t ret = this->m_rspConnector->HostFileIO(RspData("vFile:setfs:0"), output, error); + int32_t ret = connector->HostFileIO(RspData("vFile:setfs:0"), output, error); if (ret < 0) { LogDebug("Could not set remote filesystem"); @@ -765,7 +773,7 @@ std::string GdbAdapter::GetRemoteFile(const std::string& path) for ( const auto& ch : path ) path_hex_string += fmt::format("{:02X}", ch); - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:open:{},{:X},{:X}", path_hex_string.c_str(), 0, 0), output, error); if (ret < 0) { @@ -781,7 +789,7 @@ std::string GdbAdapter::GetRemoteFile(const std::string& path) while(true) { - ret = this->m_rspConnector->HostFileIO( + ret = connector->HostFileIO( RspData("vFile:pread:{:X},{:X},{:X}", fd, blockSize, offset), output, error); if (ret < 0) { @@ -805,7 +813,7 @@ std::string GdbAdapter::GetRemoteFile(const std::string& path) offset += output.AsString().length(); } - ret = this->m_rspConnector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); + ret = connector->HostFileIO(RspData(fmt::format("vFile:close:{:X}", fd)), output, error); if (ret) { auto msg = fmt::format("host i/o close() failed, result={}, errno={}", ret, error); @@ -824,7 +832,7 @@ std::vector GdbAdapter::GetModuleList() if (m_isTargetRunning) return {}; - if (!m_rspConnector) + if (!m_rspConnector.load()) return {}; std::map moduleRanges; @@ -898,11 +906,12 @@ std::string GdbAdapter::GetTargetArchitecture() bool GdbAdapter::BreakInto() { - if (!m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (!m_isTargetRunning || !connector) return false; char var = '\x03'; - this->m_rspConnector->SendRaw(RspData(&var, sizeof(var))); + connector->SendRaw(RspData(&var, sizeof(var))); m_isTargetRunning = false; return true; } @@ -910,12 +919,13 @@ bool GdbAdapter::BreakInto() DebugStopReason GdbAdapter::ResponseHandler(bool notifyStopped) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return InternalError; while (true) { - const RspData reply = m_rspConnector->ReceiveRspData(); + const RspData reply = connector->ReceiveRspData(); if (reply[0] == 'T') { // Target stopped @@ -983,11 +993,7 @@ DebugStopReason GdbAdapter::ResponseHandler(bool notifyStopped) this->m_socket->Kill(); m_isTargetRunning = false; - if (m_rspConnector) - { - delete m_rspConnector; - m_rspConnector = nullptr; - } + m_rspConnector.store(nullptr); return DebugStopReason::ProcessExited; break; @@ -1046,13 +1052,14 @@ DebugStopReason GdbAdapter::ResponseHandler(bool notifyStopped) // this should return the information about the target stop DebugStopReason GdbAdapter::GenericGo(const std::string& goCommand, bool notifyStopped) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return InternalError; m_isTargetRunning = true; // TODO: these two calls should be combined - m_rspConnector->SendPayload(RspData(goCommand)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(goCommand)); + connector->ExpectAck(); return ResponseHandler(notifyStopped); } @@ -1219,7 +1226,8 @@ bool GdbAdapter::StepOverReverse() bool GdbAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) { // Cache the hardware breakpoint to be applied when target stops or connector becomes available PendingHardwareBreakpoint pending(address, type, size); @@ -1235,7 +1243,7 @@ bool GdbAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType typ switch (type) { case HardwareExecuteBreakpoint: - // Z1 = hardware execution breakpoint + // Z1 = hardware execution breakpoint command = fmt::format("Z1,{:x},{}", address, size); break; case HardwareReadBreakpoint: @@ -1254,13 +1262,14 @@ bool GdbAdapter::AddHardwareBreakpoint(uint64_t address, DebugBreakpointType typ return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } bool GdbAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointType type, size_t size) { - if (m_isTargetRunning || !m_rspConnector) + auto connector = m_rspConnector.load(); + if (m_isTargetRunning || !connector) { // Remove from pending list if target is running or connector not available PendingHardwareBreakpoint pending(address, type, size); @@ -1277,7 +1286,7 @@ bool GdbAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointType switch (type) { case HardwareExecuteBreakpoint: - // z1 = remove hardware execution breakpoint + // z1 = remove hardware execution breakpoint command = fmt::format("z1,{:x},{}", address, size); break; case HardwareReadBreakpoint: @@ -1296,7 +1305,7 @@ bool GdbAdapter::RemoveHardwareBreakpoint(uint64_t address, DebugBreakpointType return false; } - return m_rspConnector->TransmitAndReceive(RspData(command)).AsString() == "OK"; + return connector->TransmitAndReceive(RspData(command)).AsString() == "OK"; } @@ -1370,7 +1379,8 @@ bool GdbAdapter::StepReturnReverse() std::string GdbAdapter::InvokeBackendCommand(const std::string& command) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; if (command.substr(0, 4) == "mon ") @@ -1378,7 +1388,7 @@ std::string GdbAdapter::InvokeBackendCommand(const std::string& command) else if (command.substr(0, 8) == "monitor ") return RunMonitorCommand(command.substr(8)); - auto reply = this->m_rspConnector->TransmitAndReceive(RspData(command)); + auto reply = connector->TransmitAndReceive(RspData(command)); return reply.AsString(); } @@ -1412,7 +1422,8 @@ static std::string HexToAscii(const std::string& hex) std::string GdbAdapter::RunMonitorCommand(const std::string& command) { - if (!m_rspConnector) + auto connector = m_rspConnector.load(); + if (!connector) return {}; std::string commandToSend = "qRcmd,"; @@ -1422,13 +1433,13 @@ std::string GdbAdapter::RunMonitorCommand(const std::string& command) commandToSend += ("0123456789abcdef"[c & 0x0F]); } - m_rspConnector->SendPayload(RspData(commandToSend)); - m_rspConnector->ExpectAck(); + connector->SendPayload(RspData(commandToSend)); + connector->ExpectAck(); std::string result; while (true) { - auto replyChunk = this->m_rspConnector->ReceiveRspData(); + auto replyChunk = connector->ReceiveRspData(); if (replyChunk.AsString() == "OK" || replyChunk.AsString().empty()) break; diff --git a/core/adapters/gdbadapter.h b/core/adapters/gdbadapter.h index 5d9896b1..c8dd4675 100644 --- a/core/adapters/gdbadapter.h +++ b/core/adapters/gdbadapter.h @@ -47,7 +47,7 @@ namespace BinaryNinjaDebugger using register_pair = std::pair; Socket* m_socket; - RspConnector* m_rspConnector{}; + AtomicRspConnector m_rspConnector; std::map m_registerInfo{}; std::optional> m_regCache; diff --git a/core/adapters/rspconnector.h b/core/adapters/rspconnector.h index 2c9c7bb1..b0570c07 100644 --- a/core/adapters/rspconnector.h +++ b/core/adapters/rspconnector.h @@ -15,6 +15,8 @@ limitations under the License. */ #pragma once +#include +#include #include #include #include @@ -202,4 +204,27 @@ namespace BinaryNinjaDebugger std::vector GetServerCapabilities() const { return m_serverCapabilities; } }; + + // Holds a std::shared_ptr with thread-safe load/store semantics. + // Adapter callers like BreakInto run on a thread that races with the adapter's own + // ResponseHandler, which can destroy the connector on target exit. Reads take a + // strong reference under the lock, so the connector cannot be freed while in use. + class AtomicRspConnector + { + mutable std::mutex m_mutex; + std::shared_ptr m_ptr; + + public: + std::shared_ptr load() const + { + std::lock_guard lock(m_mutex); + return m_ptr; + } + + void store(std::shared_ptr ptr) + { + std::lock_guard lock(m_mutex); + m_ptr = std::move(ptr); + } + }; };