From eef8ee6a2ecf6caf2ce20c9358181fec13f0bb3e Mon Sep 17 00:00:00 2001 From: David Young Date: Tue, 19 May 2026 07:31:16 -0700 Subject: [PATCH 1/2] Fix use after free in MCP --- lldb/include/lldb/Host/JSONTransport.h | 5 +++-- lldb/unittests/Host/JSONTransportTest.cpp | 18 ++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h index 6b114ee497a8b..4a27c307d0c98 100644 --- a/lldb/include/lldb/Host/JSONTransport.h +++ b/lldb/include/lldb/Host/JSONTransport.h @@ -259,9 +259,10 @@ template class IOTransport : public JSONTransport { if (!m_buffer.empty()) handler.OnError(llvm::make_error( std::string(m_buffer.str()))); - handler.OnClosed(); - // On EOF, remove the read handle from the MainLoop. + // Remove the read handle from the MainLoop before notifying the handler, + // because OnClosed may destroy this transport (and the handler). m_read_handle.reset(); + handler.OnClosed(); } } 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. From 1e61239ed9f0f0f9cac23a84ecbbb2b35310b1a8 Mon Sep 17 00:00:00 2001 From: David Young Date: Tue, 19 May 2026 08:06:35 -0700 Subject: [PATCH 2/2] Move to a local variable --- lldb/include/lldb/Host/JSONTransport.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lldb/include/lldb/Host/JSONTransport.h b/lldb/include/lldb/Host/JSONTransport.h index 4a27c307d0c98..c3a90cb0c3364 100644 --- a/lldb/include/lldb/Host/JSONTransport.h +++ b/lldb/include/lldb/Host/JSONTransport.h @@ -259,9 +259,10 @@ template class IOTransport : public JSONTransport { if (!m_buffer.empty()) handler.OnError(llvm::make_error( std::string(m_buffer.str()))); - // Remove the read handle from the MainLoop before notifying the handler, - // because OnClosed may destroy this transport (and the handler). - m_read_handle.reset(); + // 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(); } }