From 60f27523c69c8d0c8eb9b2a78920b68f3cacffd0 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 12 Jun 2026 16:30:54 +0200 Subject: [PATCH] LibIPC: Adopt Mach OOL payloads on receive Mach transport already sends payloads as out-of-line virtual-copy regions, but the receive path immediately copied each payload into a new Vector and deallocated the kernel mapping. That made the IPC IO thread touch every byte before the main thread could decode the message. Add ReceivedMessageBytes as the raw-message byte storage and let the Mach transport adopt the OOL region directly. The mapping now lives until the raw message storage is destroyed, so invalid descriptor paths and normal queue teardown both release it through the same destructor. Socket transports keep their existing receive copy path by wrapping vectors in the same storage type, and the direct raw-message consumers now decode from its ReadonlyBytes view. --- Libraries/LibIPC/CMakeLists.txt | 1 + Libraries/LibIPC/Connection.cpp | 5 +- Libraries/LibIPC/ReceivedMessageBytes.cpp | 100 ++++++++++++++++++++ Libraries/LibIPC/ReceivedMessageBytes.h | 41 ++++++++ Libraries/LibIPC/TransportMachPort.cpp | 13 +-- Libraries/LibIPC/TransportMachPort.h | 3 +- Libraries/LibIPC/TransportSocket.cpp | 4 +- Libraries/LibIPC/TransportSocket.h | 3 +- Libraries/LibIPC/TransportSocketWindows.cpp | 4 +- Libraries/LibIPC/TransportSocketWindows.h | 3 +- Libraries/LibWeb/HTML/MessagePort.cpp | 2 +- 11 files changed, 162 insertions(+), 17 deletions(-) create mode 100644 Libraries/LibIPC/ReceivedMessageBytes.cpp create mode 100644 Libraries/LibIPC/ReceivedMessageBytes.h diff --git a/Libraries/LibIPC/CMakeLists.txt b/Libraries/LibIPC/CMakeLists.txt index 62663b545b..0970926585 100644 --- a/Libraries/LibIPC/CMakeLists.txt +++ b/Libraries/LibIPC/CMakeLists.txt @@ -5,6 +5,7 @@ set(SOURCES Encoder.cpp File.cpp Message.cpp + ReceivedMessageBytes.cpp TransportHandle.cpp ) diff --git a/Libraries/LibIPC/Connection.cpp b/Libraries/LibIPC/Connection.cpp index b2bbad4e56..91f1d41846 100644 --- a/Libraries/LibIPC/Connection.cpp +++ b/Libraries/LibIPC/Connection.cpp @@ -97,10 +97,11 @@ ConnectionBase::PeerEOF ConnectionBase::drain_messages_from_peer() { bool parse_error = false; auto schedule_shutdown = m_transport->read_as_many_messages_as_possible_without_blocking([&](auto&& raw_message) { - if (auto message = try_parse_message(raw_message.bytes, raw_message.attachments)) { + auto bytes = raw_message.bytes.bytes(); + if (auto message = try_parse_message(bytes, raw_message.attachments)) { m_unprocessed_messages.append(message.release_nonnull()); } else { - dbgln("Failed to parse IPC message {:hex-dump}", raw_message.bytes); + dbgln("Failed to parse IPC message {:hex-dump}", bytes); parse_error = true; } }); diff --git a/Libraries/LibIPC/ReceivedMessageBytes.cpp b/Libraries/LibIPC/ReceivedMessageBytes.cpp new file mode 100644 index 0000000000..0df0f0d833 --- /dev/null +++ b/Libraries/LibIPC/ReceivedMessageBytes.cpp @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2026, Aliaksandr Kalenik + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#include +#include + +#if defined(AK_OS_MACOS) +# include +#endif + +namespace IPC { + +class ReceivedMessageBytes::Impl final : public RefCounted { +public: + explicit Impl(Vector bytes) + : m_storage_type(StorageType::Vector) + , m_vector(move(bytes)) + { + } + +#if defined(AK_OS_MACOS) + Impl(void* vm_region_address, size_t vm_region_size) + : m_storage_type(StorageType::VMRegion) + , m_vm_region_address(vm_region_address) + , m_vm_region_size(vm_region_size) + { + } +#endif + + ~Impl() + { +#if defined(AK_OS_MACOS) + if (m_storage_type == StorageType::VMRegion && m_vm_region_size > 0) + vm_deallocate(mach_task_self(), reinterpret_cast(m_vm_region_address), m_vm_region_size); +#endif + } + + ReadonlyBytes bytes() const + { + switch (m_storage_type) { + case StorageType::Vector: + return m_vector; + case StorageType::VMRegion: + return { static_cast(m_vm_region_address), m_vm_region_size }; + } + VERIFY_NOT_REACHED(); + } + +private: + enum class StorageType { + Vector, + VMRegion, + }; + + StorageType m_storage_type { StorageType::Vector }; + Vector m_vector; + void* m_vm_region_address { nullptr }; + size_t m_vm_region_size { 0 }; +}; + +ReceivedMessageBytes::ReceivedMessageBytes() = default; +ReceivedMessageBytes::ReceivedMessageBytes(ReceivedMessageBytes const&) = default; +ReceivedMessageBytes::ReceivedMessageBytes(ReceivedMessageBytes&&) = default; +ReceivedMessageBytes::~ReceivedMessageBytes() = default; + +ReceivedMessageBytes& ReceivedMessageBytes::operator=(ReceivedMessageBytes const&) = default; +ReceivedMessageBytes& ReceivedMessageBytes::operator=(ReceivedMessageBytes&&) = default; + +ReceivedMessageBytes::ReceivedMessageBytes(NonnullRefPtr impl) + : m_impl(move(impl)) +{ +} + +ReceivedMessageBytes ReceivedMessageBytes::from_vector(Vector bytes) +{ + if (bytes.is_empty()) + return {}; + return ReceivedMessageBytes { adopt_ref(*new Impl(move(bytes))) }; +} + +#if defined(AK_OS_MACOS) +ReceivedMessageBytes ReceivedMessageBytes::adopt_vm_region(void* address, size_t size) +{ + if (size == 0) + return {}; + return ReceivedMessageBytes { adopt_ref(*new Impl(address, size)) }; +} +#endif + +ReadonlyBytes ReceivedMessageBytes::bytes() const +{ + if (!m_impl) + return {}; + return m_impl->bytes(); +} + +} diff --git a/Libraries/LibIPC/ReceivedMessageBytes.h b/Libraries/LibIPC/ReceivedMessageBytes.h new file mode 100644 index 0000000000..0af6c53be4 --- /dev/null +++ b/Libraries/LibIPC/ReceivedMessageBytes.h @@ -0,0 +1,41 @@ +/* + * Copyright (c) 2026, Aliaksandr Kalenik + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include +#include + +namespace IPC { + +class ReceivedMessageBytes { +public: + ReceivedMessageBytes(); + ReceivedMessageBytes(ReceivedMessageBytes const&); + ReceivedMessageBytes(ReceivedMessageBytes&&); + ~ReceivedMessageBytes(); + + ReceivedMessageBytes& operator=(ReceivedMessageBytes const&); + ReceivedMessageBytes& operator=(ReceivedMessageBytes&&); + + static ReceivedMessageBytes from_vector(Vector); +#if defined(AK_OS_MACOS) + static ReceivedMessageBytes adopt_vm_region(void*, size_t); +#endif + + ReadonlyBytes bytes() const; + bool is_empty() const { return bytes().is_empty(); } + +private: + class Impl; + + explicit ReceivedMessageBytes(NonnullRefPtr); + + RefPtr m_impl; +}; + +} diff --git a/Libraries/LibIPC/TransportMachPort.cpp b/Libraries/LibIPC/TransportMachPort.cpp index aeefed2f19..6d21be32c7 100644 --- a/Libraries/LibIPC/TransportMachPort.cpp +++ b/Libraries/LibIPC/TransportMachPort.cpp @@ -297,27 +297,22 @@ void TransportMachPort::process_received_message(u8* buffer) auto* descriptors = reinterpret_cast(body + 1); auto const* payload = reinterpret_cast(&descriptors[attachment_count]); + VERIFY(payload->type == MACH_MSG_OOL_DESCRIPTOR); + auto payload_bytes = ReceivedMessageBytes::adopt_vm_region(payload->address, payload->size); + auto message = make(); for (unsigned int i = 0; i < attachment_count; ++i) { auto attachment = attachment_from_descriptor(descriptors[i]); if (attachment.is_error()) { dbgln("TransportMachPort: dropping message with invalid port descriptor {} of {}: {} (name={:x}, disposition={}, type={})", i + 1, attachment_count, attachment.error(), descriptors[i].name, descriptors[i].disposition, descriptors[i].type); - if (payload->type == MACH_MSG_OOL_DESCRIPTOR && payload->size > 0) - vm_deallocate(mach_task_self(), reinterpret_cast(payload->address), payload->size); mark_peer_eof(); return; } message->attachments.enqueue(attachment.release_value()); } - VERIFY(payload->type == MACH_MSG_OOL_DESCRIPTOR); - if (payload->size > 0) { - message->bytes.append(static_cast(payload->address), payload->size); - // The out-of-line payload arrives as a temporary mapping in this task. After copying it into our queue, - // unmap that region so each receive does not leak virtual memory. - vm_deallocate(mach_task_self(), reinterpret_cast(payload->address), payload->size); - } + message->bytes = move(payload_bytes); if (message->bytes.is_empty() && message->attachments.is_empty()) return; diff --git a/Libraries/LibIPC/TransportMachPort.h b/Libraries/LibIPC/TransportMachPort.h index 4d74b93219..91bddeed0a 100644 --- a/Libraries/LibIPC/TransportMachPort.h +++ b/Libraries/LibIPC/TransportMachPort.h @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -57,7 +58,7 @@ public: Yes, }; struct Message { - Vector bytes; + ReceivedMessageBytes bytes; Queue attachments; }; ShouldShutdown read_as_many_messages_as_possible_without_blocking(Function&&); diff --git a/Libraries/LibIPC/TransportSocket.cpp b/Libraries/LibIPC/TransportSocket.cpp index 2076866a60..5a9525d9db 100644 --- a/Libraries/LibIPC/TransportSocket.cpp +++ b/Libraries/LibIPC/TransportSocket.cpp @@ -448,11 +448,13 @@ void TransportSocket::read_incoming_messages() } for (size_t i = 0; i < header.fd_count; ++i) message->attachments.enqueue(m_unprocessed_attachments.dequeue()); - if (message->bytes.try_append(m_unprocessed_bytes.data() + index + sizeof(SocketMessageHeader), header.payload_size).is_error()) { + Vector payload_bytes; + if (payload_bytes.try_append(m_unprocessed_bytes.data() + index + sizeof(SocketMessageHeader), header.payload_size).is_error()) { dbgln("TransportSocket: Failed to allocate message buffer for payload_size {}", header.payload_size); m_peer_eof = true; break; } + message->bytes = ReceivedMessageBytes::from_vector(move(payload_bytes)); batch.append(move(message)); } else if (header.type == SocketMessageHeader::Type::FileDescriptorAcknowledgement) { if (header.payload_size != 0) { diff --git a/Libraries/LibIPC/TransportSocket.h b/Libraries/LibIPC/TransportSocket.h index 3fad3d1018..ec921ea983 100644 --- a/Libraries/LibIPC/TransportSocket.h +++ b/Libraries/LibIPC/TransportSocket.h @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -88,7 +89,7 @@ public: Yes, }; struct Message { - Vector bytes; + ReceivedMessageBytes bytes; Queue attachments; }; ShouldShutdown read_as_many_messages_as_possible_without_blocking(Function&&); diff --git a/Libraries/LibIPC/TransportSocketWindows.cpp b/Libraries/LibIPC/TransportSocketWindows.cpp index eb3682e5be..ad9b3bc477 100644 --- a/Libraries/LibIPC/TransportSocketWindows.cpp +++ b/Libraries/LibIPC/TransportSocketWindows.cpp @@ -296,11 +296,13 @@ TransportSocketWindows::ShouldShutdown TransportSocketWindows::read_as_many_mess VERIFY(attachment_bytes.is_empty()); auto const* payload = m_unprocessed_bytes.data() + index + sizeof(MessageHeader) + header.attachment_data_size; - if (message.bytes.try_append(payload, header.payload_size).is_error()) { + Vector payload_bytes; + if (payload_bytes.try_append(payload, header.payload_size).is_error()) { dbgln("TransportSocketWindows: Failed to allocate message buffer for payload_size {}", header.payload_size); should_shutdown = ShouldShutdown::Yes; break; } + message.bytes = ReceivedMessageBytes::from_vector(move(payload_bytes)); callback(move(message)); Checked new_index = index; new_index += header.payload_size; diff --git a/Libraries/LibIPC/TransportSocketWindows.h b/Libraries/LibIPC/TransportSocketWindows.h index e52116e545..388bbc68b4 100644 --- a/Libraries/LibIPC/TransportSocketWindows.h +++ b/Libraries/LibIPC/TransportSocketWindows.h @@ -11,6 +11,7 @@ #include #include #include +#include #include namespace IPC { @@ -44,7 +45,7 @@ public: Yes, }; struct Message { - Vector bytes; + ReceivedMessageBytes bytes; Queue attachments; }; ShouldShutdown read_as_many_messages_as_possible_without_blocking(Function&&); diff --git a/Libraries/LibWeb/HTML/MessagePort.cpp b/Libraries/LibWeb/HTML/MessagePort.cpp index 19ef5e2649..cfbde3411c 100644 --- a/Libraries/LibWeb/HTML/MessagePort.cpp +++ b/Libraries/LibWeb/HTML/MessagePort.cpp @@ -351,7 +351,7 @@ void MessagePort::drain_transport() return; auto schedule_shutdown = m_transport->read_as_many_messages_as_possible_without_blocking([this](auto&& raw_message) { - FixedMemoryStream stream { raw_message.bytes.span(), FixedMemoryStream::Mode::ReadOnly }; + FixedMemoryStream stream { raw_message.bytes.bytes() }; IPC::Decoder decoder { stream, raw_message.attachments }; m_pending_incoming_messages.append(MUST(decoder.decode()));