Fix use-after-free in IOTransport::OnRead on client disconnect

[youngd007]

When an MCP client disconnects (EOF), IOTransport::OnRead called
handler.OnClosed() before resetting m_read_handle. The MCP server's
OnClosed handler erases the client from m_instances, destroying both
the transport (this) and the binder (handler). The subsequent
m_read_handle.reset() then accessed the destroyed transport's member,
causing a use-after-free (SIGSEGV).

• thread Fixing Rust build #1, stop reason = signal SIGSEGV: address not mapped to object (fault address=0x28)

  • frame #0: 0x00007ff5d4d5afda liblldb.so.23.2lldb_private::transport::IOTransport<lldb_protocol::mcp::ProtocolDescriptor>::OnRead(lldb_private::MainLoopBase&, lldb_private::transport::JSONTransport<lldb_protocol::mcp::ProtocolDescriptor>::MessageHandler&) + 1274 frame #1: 0x00007ff5d1140ad8 liblldb.so.23.0lldb_private::MainLoopPosix::Run() + 408
    frame Fix a typo #2: 0x00007ff5d1760c1c liblldb.so.23.0`std::thread::_State_impl<std::thre

  Fix by resetting the read handle before calling OnClosed(), so no
  transport members are accessed after the handler potentially destroys
  the transport.

Then when the scope is left, the destructor is called for the new read_handle local variable and it is cleaned up.

New unit tests added that fail without this change. With the change, the custom 'ai' script (allows end user locally to communicate lldb context to agent backend via a spun up MCP server: "protocol-server start MCP listen://localhost:{port}") now successfully concludes without this crash

Assisted with: claude

[llvmorg-github-actions]

@llvm/pr-subscribers-lldb

Author: youngd007

Changes

When an MCP client disconnects (EOF), IOTransport::OnRead called
handler.OnClosed() before resetting m_read_handle. The MCP server's
OnClosed handler erases the client from m_instances, destroying both
the transport (this) and the binder (handler). The subsequent
m_read_handle.reset() then accessed the destroyed transport's member,
causing a use-after-free (SIGSEGV).

• thread #1, stop reason = signal SIGSEGV: address not mapped to object (fault address=0x28)

  • frame #0: 0x00007ff5d4d5afda liblldb.so.23.2lldb_private::transport::IOTransport&lt;lldb_protocol::mcp::ProtocolDescriptor&gt;::OnRead(lldb_private::MainLoopBase&amp;, lldb_private::transport::JSONTransport&lt;lldb_protocol::mcp::ProtocolDescriptor&gt;::MessageHandler&amp;) + 1274 frame #<!-- -->1: 0x00007ff5d1140ad8 liblldb.so.23.0lldb_private::MainLoopPosix::Run() + 408
    frame #2: 0x00007ff5d1760c1c liblldb.so.23.0`std::thread::_State_impl<std::thre

  Fix by resetting the read handle before calling OnClosed(), so no
  transport members are accessed after the handler potentially destroys
  the transport.

Then when the scope is left, the destructor is called for the new read_handle local variable and it is cleaned up.

New unit tests added that fail without this change. With the change, the custom 'ai' script (allows end user locally to communicate lldb context to agent backend via a spun up MCP server: "protocol-server start MCP listen://localhost:{port}") now successfully concludes without this crash

Assisted with: claude

---

Full diff: https://github.com/llvm/llvm-project/pull/198548.diff

2 Files Affected:

• (modified) lldb/include/lldb/Host/JSONTransport.h (+4-2)
• (modified) lldb/unittests/Host/JSONTransportTest.cpp (+18)

diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h
index 6b114ee497a8b..c3a90cb0c3364 100644
--- a/lldb/include/lldb/Host/JSONTransport.h
+++ b/lldb/include/lldb/Host/JSONTransport.h
@@ -259,9 +259,11 @@ template <typename Proto> class IOTransport : public JSONTransport<Proto> {
       if (!m_buffer.empty())
         handler.OnError(llvm::make_error<TransportUnhandledContentsError>(
             std::string(m_buffer.str())));
+      // Move the read handle to a local before notifying the handler. The
+      // handler may destroy this transport (e.g. by erasing it from a
+      // connection map), so accessing members after OnClosed() is unsafe.
+      auto read_handle = std::move(m_read_handle);
       handler.OnClosed();
-      // On EOF, remove the read handle from the MainLoop.
-      m_read_handle.reset();
     }
   }
 
diff --git a/lldb/unittests/Host/JSONTransportTest.cpp b/lldb/unittests/Host/JSONTransportTest.cpp
index 2c26f94213773..3e977a70f5d4e 100644
--- a/lldb/unittests/Host/JSONTransportTest.cpp
+++ b/lldb/unittests/Host/JSONTransportTest.cpp
@@ -479,6 +479,15 @@ TEST_F(HTTPDelimitedJSONTransportTest, ReadWithEOF) {
   ASSERT_THAT_ERROR(Run(), Succeeded());
 }
 
+TEST_F(HTTPDelimitedJSONTransportTest, ReadWithEOFDestroyTransportOnClose) {
+  input.CloseWriteFileDescriptor();
+  EXPECT_CALL(message_handler, OnClosed()).WillOnce([this]() {
+    transport.reset();
+    loop.RequestTermination();
+  });
+  ASSERT_THAT_ERROR(loop.Run().takeError(), Succeeded());
+}
+
 TEST_F(HTTPDelimitedJSONTransportTest, ReaderWithUnhandledData) {
   std::string json = R"json({"str": "foo"})json";
   std::string message =
@@ -588,6 +597,15 @@ TEST_F(JSONRPCTransportTest, ReadWithEOF) {
   ASSERT_THAT_ERROR(Run(), Succeeded());
 }
 
+TEST_F(JSONRPCTransportTest, ReadWithEOFDestroyTransportOnClose) {
+  input.CloseWriteFileDescriptor();
+  EXPECT_CALL(message_handler, OnClosed()).WillOnce([this]() {
+    transport.reset();
+    loop.RequestTermination();
+  });
+  ASSERT_THAT_ERROR(loop.Run().takeError(), Succeeded());
+}
+
 TEST_F(JSONRPCTransportTest, ReaderWithUnhandledData) {
   std::string message = R"json({"req": "foo")json";
   // Write an incomplete message and close the handle.

[JDevlieghere]

Thanks!