From 03dcfd3931b28a8503d3949d61cf21bf349f2809 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Rydg=C3=A5rd?= Date: Sat, 8 Aug 2026 13:56:51 +0200 Subject: [PATCH] WebSocket debugger: fail fast on invalid memory ranges, move base64 off the CPU thread memory.read_u8/u16/u32/read/readString/write_u8/u16/u32/write/search all validated their address/size parameters (and, for search, the rest of its param parsing) after already queuing onto the CPU thread. None of that depends on CPU-thread-owned state, so do it upfront instead and fail fast without a round trip through the queue for requests we already know are invalid. Also, for memory.read and memory.readString, only the raw memory copy (which needs replacements/emuhacks disabled) now happens on the CPU thread - the base64 encoding itself happens back on the WebSocket thread afterward, so a large read no longer blocks the CPU thread's frame pump for the encoding work too. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH --- Core/Debugger/WebSocket/MemorySubscriber.cpp | 397 +++++++++---------- 1 file changed, 186 insertions(+), 211 deletions(-) diff --git a/Core/Debugger/WebSocket/MemorySubscriber.cpp b/Core/Debugger/WebSocket/MemorySubscriber.cpp index 265cbb9926..7d61ee5d14 100644 --- a/Core/Debugger/WebSocket/MemorySubscriber.cpp +++ b/Core/Debugger/WebSocket/MemorySubscriber.cpp @@ -112,20 +112,17 @@ void WebSocketMemoryReadU8(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory read to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } - JsonWriter &json = req.Respond(); json.writeUint("value", Memory::Read_U8(addr)); }); @@ -144,20 +141,17 @@ void WebSocketMemoryReadU16(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory read to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } - JsonWriter &json = req.Respond(); json.writeUint("value", Memory::Read_U16(addr)); }); @@ -176,20 +170,17 @@ void WebSocketMemoryReadU32(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory read to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } - JsonWriter &json = req.Respond(); json.writeUint("value", Memory::Read_U32(addr)); }); @@ -215,40 +206,41 @@ void WebSocketMemoryRead(DebuggerRequest &req) { if (!req.ParamBool("replacements", &replacements, DebuggerParamType::OPTIONAL)) return; - // Route the actual memory read to the CPU thread instead of poking at it directly - // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. - // Note: for a very large 'size', base64-encoding it is real CPU work that will now happen on the - // CPU thread itself, blocking its own frame pump for the duration - same caveat as memory.search. + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr/size, not on anything CPU-thread-owned, so fail fast here rather + // than making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + if (!Memory::IsValidRange(addr, size)) + return req.Fail("Invalid size"); + + // Route the actual memory read to the CPU thread instead of poking at it directly from this + // WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Only the raw copy (which + // needs replacements/emuhacks disabled) happens on the CPU thread - the base64 encoding itself + // happens back on this WebSocket thread afterward, so it doesn't block the CPU thread's frame + // pump for a large 'size'. + std::vector raw(size); Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(replacements); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } else if (!Memory::IsValidRange(addr, size)) { - req.Fail("Invalid size"); - return; - } - - JsonWriter &json = req.Respond(); - // Start a value without any actual data yet... - json.writeRaw("base64", ""); - req.Flush(); - - // Now we'll write it directly to the stream. - req.ws->AddFragment(false, "\""); - // 65535 is an "even" number of base64 characters. - static const size_t CHUNK_SIZE = 65535; - for (size_t i = 0; i < size; i += CHUNK_SIZE) { - size_t left = std::min(size - i, CHUNK_SIZE); - req.ws->AddFragment(false, Base64Encode(Memory::GetPointerUnchecked(addr) + i, left)); - } - req.ws->AddFragment(false, "\""); + if (size != 0) + memcpy(raw.data(), Memory::GetPointerUnchecked(addr), size); }); + + JsonWriter &json = req.Respond(); + // Start a value without any actual data yet... + json.writeRaw("base64", ""); + req.Flush(); + + // Now we'll write it directly to the stream. + req.ws->AddFragment(false, "\""); + // 65535 is an "even" number of base64 characters. + static const size_t CHUNK_SIZE = 65535; + for (size_t i = 0; i < raw.size(); i += CHUNK_SIZE) { + size_t left = std::min(raw.size() - i, CHUNK_SIZE); + req.ws->AddFragment(false, Base64Encode(&raw[i], left)); + } + req.ws->AddFragment(false, "\""); } // Read a NUL terminated string from memory (memory.readString) @@ -267,40 +259,39 @@ void WebSocketMemoryReadString(DebuggerRequest &req) { if (!req.ParamU32("address", &addr)) return; - // Route the actual memory read to the CPU thread instead of poking at it directly - // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + + std::string type = "utf-8"; + if (!req.ParamString("type", &type, DebuggerParamType::OPTIONAL)) + return; + if (type != "utf-8" && type != "base64") + return req.Fail("Invalid type, must be either utf-8 or base64"); + + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + + // Route the actual memory read to the CPU thread instead of poking at it directly from this + // WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Only the raw copy happens on + // the CPU thread - the base64 encoding itself happens back on this WebSocket thread afterward. + std::string raw; Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - std::string type = "utf-8"; - if (!req.ParamString("type", &type, DebuggerParamType::OPTIONAL)) - return; - if (type != "utf-8" && type != "base64") { - req.Fail("Invalid type, must be either utf-8 or base64"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } - // Let's try to avoid crashing and get a safe length. const uint8_t *p = Memory::GetPointerUnchecked(addr); size_t longest = Memory::ClampValidSizeAt(addr, Memory::g_MemorySize); size_t len = strnlen((const char *)p, longest); - - JsonWriter &json = req.Respond(); - if (type == "utf-8") { - json.writeString("value", std::string((const char *)p, len)); - } else if (type == "base64") { - json.writeString("base64", Base64Encode(p, len)); - } + raw.assign((const char *)p, len); }); + + JsonWriter &json = req.Respond(); + if (type == "utf-8") { + json.writeString("value", raw); + } else if (type == "base64") { + json.writeString("base64", Base64Encode((const uint8_t *)raw.data(), raw.size())); + } } // Write a byte to memory (memory.write_u8) @@ -320,19 +311,17 @@ void WebSocketMemoryWriteU8(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory write to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } currentMIPS->InvalidateICache(addr, 1); Memory::Write_U8(val, addr); Reporting::NotifyDebugger(); @@ -359,19 +348,17 @@ void WebSocketMemoryWriteU16(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory write to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } currentMIPS->InvalidateICache(addr, 2); Memory::Write_U16(val, addr); Reporting::NotifyDebugger(); @@ -398,19 +385,17 @@ void WebSocketMemoryWriteU32(DebuggerRequest &req) { return; } + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr, not on anything CPU-thread-owned, so fail fast here rather than + // making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + // Route the actual memory write to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } currentMIPS->InvalidateICache(addr, 4); Memory::Write_U32(val, addr); Reporting::NotifyDebugger(); @@ -435,28 +420,26 @@ void WebSocketMemoryWrite(DebuggerRequest &req) { if (!req.ParamString("base64", &encoded)) return; + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + + // Decoding doesn't touch anything CPU-thread-owned either, so do it - and the address/size + // validation that depends on it - here, before we bother queuing to the CPU thread at all. + std::vector value = Base64Decode(&encoded[0], encoded.size()); + uint32_t size = (uint32_t)value.size(); + + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + if (value.size() != (size_t)size || !Memory::IsValidRange(addr, size)) + return req.Fail("Invalid size"); + // Route the actual memory write to the CPU thread instead of poking at it directly // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - - std::vector value = Base64Decode(&encoded[0], encoded.size()); - uint32_t size = (uint32_t)value.size(); - - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } else if (value.size() != (size_t)size || !Memory::IsValidRange(addr, size)) { - req.Fail("Invalid size"); - return; - } - currentMIPS->InvalidateICache(addr, size); - Memory::MemcpyUnchecked(addr, &value[0], size); + if (size != 0) + Memory::MemcpyUnchecked(addr, &value[0], size); Reporting::NotifyDebugger(); req.Respond(); }); @@ -498,87 +481,79 @@ void WebSocketMemorySearch(DebuggerRequest &req) { if (!req.ParamString("type", &type)) return; - // Route the actual memory scan to the CPU thread instead of poking at it directly - // from this WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. - // Note: 'size' has no cap beyond valid memory range, so a very large scan will now block the CPU - // thread's own frame pump for its duration, rather than running unqueued on this thread as before. + if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) + return req.Fail("CPU not started"); + // This only depends on addr/size, not on anything CPU-thread-owned, so fail fast here rather + // than making a round trip through the queue for a request we already know is invalid. + if (!Memory::IsValidAddress(addr)) + return req.Fail("Invalid address"); + if (!Memory::IsValidRange(addr, size)) + return req.Fail("Invalid size"); + + // None of the rest of the parameter parsing/validation below touches anything CPU-thread-owned + // either, so it all happens here too, before we bother queuing to the CPU thread at all. + uint32_t align = 1; + uint32_t needleSize = 0; + uint32_t needleValue = 0; + std::vector needleBytes; + std::vector maskBytes; + + if (type == "u8" || type == "u16" || type == "u32") { + needleSize = type == "u8" ? 1 : type == "u16" ? 2 : 4; + align = needleSize; + if (!req.ParamU32("value", &needleValue, false)) + return; + } else if (type == "float") { + needleSize = 4; + align = 4; + // allowFloatBits: accepts a string like "1.5" and gives us its raw bit pattern. + if (!req.ParamU32("value", &needleValue, true)) + return; + } else if (type == "bytes") { + std::string encoded; + if (!req.ParamString("base64", &encoded)) + return; + needleBytes = Base64Decode(&encoded[0], encoded.size()); + if (needleBytes.empty()) + return req.Fail("'base64' must decode to at least one byte"); + needleSize = (uint32_t)needleBytes.size(); + align = 1; + + if (req.HasParam("maskBase64")) { + std::string maskEncoded; + if (!req.ParamString("maskBase64", &maskEncoded)) + return; + maskBytes = Base64Decode(&maskEncoded[0], maskEncoded.size()); + if (maskBytes.size() != needleBytes.size()) + return req.Fail("'maskBase64' must decode to the same length as 'base64'"); + } + } else { + return req.Fail("Invalid 'type', must be u8, u16, u32, float, or bytes"); + } + + if (needleSize > size) + return req.Fail("'size' is smaller than the pattern/value being searched for"); + + if (!req.ParamU32("align", &align, false, DebuggerParamType::OPTIONAL)) + return; + if (align == 0) + return req.Fail("'align' must not be zero"); + + uint32_t maxResults = 1000; + if (!req.ParamU32("maxResults", &maxResults, false, DebuggerParamType::OPTIONAL)) + return; + if (maxResults == 0) + maxResults = 1000; + else if (maxResults > 100000) + maxResults = 100000; + + // Route the actual memory scan to the CPU thread instead of poking at it directly from this + // WebSocket handler thread - see Core_RunOnCPUThread() in Core.h. + // Note: 'size' has no cap beyond valid memory range, so a very large scan will still block the + // CPU thread's own frame pump for its duration - at least the parameter validation above no + // longer costs a round trip through the queue first. Core_RunOnCPUThread([&] { AutoDisabledReplacements memLock = LockMemory(true); - if (!currentDebugMIPS->isAlive() || !Memory::IsActive()) { - req.Fail("CPU not started"); - return; - } - if (!Memory::IsValidAddress(addr)) { - req.Fail("Invalid address"); - return; - } else if (!Memory::IsValidRange(addr, size)) { - req.Fail("Invalid size"); - return; - } - - uint32_t align = 1; - uint32_t needleSize = 0; - uint32_t needleValue = 0; - std::vector needleBytes; - std::vector maskBytes; - - if (type == "u8" || type == "u16" || type == "u32") { - needleSize = type == "u8" ? 1 : type == "u16" ? 2 : 4; - align = needleSize; - if (!req.ParamU32("value", &needleValue, false)) - return; - } else if (type == "float") { - needleSize = 4; - align = 4; - // allowFloatBits: accepts a string like "1.5" and gives us its raw bit pattern. - if (!req.ParamU32("value", &needleValue, true)) - return; - } else if (type == "bytes") { - std::string encoded; - if (!req.ParamString("base64", &encoded)) - return; - needleBytes = Base64Decode(&encoded[0], encoded.size()); - if (needleBytes.empty()) { - req.Fail("'base64' must decode to at least one byte"); - return; - } - needleSize = (uint32_t)needleBytes.size(); - align = 1; - - if (req.HasParam("maskBase64")) { - std::string maskEncoded; - if (!req.ParamString("maskBase64", &maskEncoded)) - return; - maskBytes = Base64Decode(&maskEncoded[0], maskEncoded.size()); - if (maskBytes.size() != needleBytes.size()) { - req.Fail("'maskBase64' must decode to the same length as 'base64'"); - return; - } - } - } else { - req.Fail("Invalid 'type', must be u8, u16, u32, float, or bytes"); - return; - } - - if (needleSize > size) { - req.Fail("'size' is smaller than the pattern/value being searched for"); - return; - } - - if (!req.ParamU32("align", &align, false, DebuggerParamType::OPTIONAL)) - return; - if (align == 0) { - req.Fail("'align' must not be zero"); - return; - } - - uint32_t maxResults = 1000; - if (!req.ParamU32("maxResults", &maxResults, false, DebuggerParamType::OPTIONAL)) - return; - if (maxResults == 0) - maxResults = 1000; - else if (maxResults > 100000) - maxResults = 100000; const uint8_t *base = Memory::GetPointerUnchecked(addr); std::vector matches;