From 0b13973563599eb2900dc1e5356a773a7d570ae8 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Sun, 5 Jul 2026 17:19:56 +0200 Subject: [PATCH 1/8] feat: Add attachment update IPC Allow clients to send replacement attachment contents to the handler with variable-length IPC messages. The handler writes the updated bytes to the attachment path while keeping attachment registration unchanged. Co-Authored-By: OpenAI Codex --- client/crashpad_client.h | 8 ++ client/crashpad_client_linux.cc | 24 ++++ client/crashpad_client_mac.cc | 24 ++++ client/crashpad_client_win.cc | 106 ++++++++++++++++-- .../linux/crash_report_exception_handler.cc | 26 +++++ .../linux/crash_report_exception_handler.h | 4 + handler/linux/exception_handler_server.cc | 59 +++++++++- handler/linux/exception_handler_server.h | 9 ++ handler/mac/crash_report_exception_handler.cc | 25 +++++ handler/mac/crash_report_exception_handler.h | 4 + handler/mac/exception_handler_server.cc | 23 ++++ handler/mac/exception_handler_server.h | 12 ++ handler/win/crash_report_exception_handler.cc | 25 +++++ handler/win/crash_report_exception_handler.h | 4 + util/linux/exception_handler_client.cc | 85 ++++++++++++++ util/linux/exception_handler_client.h | 10 ++ util/linux/exception_handler_protocol.h | 19 ++++ util/linux/socket.cc | 8 +- util/linux/socket.h | 3 +- util/mach/exception_handler_protocol.h | 4 + util/win/exception_handler_server.cc | 58 ++++++++++ util/win/exception_handler_server.h | 10 ++ util/win/registration_protocol_win.cc | 32 ++---- util/win/registration_protocol_win.h | 20 ++-- util/win/registration_protocol_win_structs.h | 26 +++++ 25 files changed, 581 insertions(+), 47 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 8067c4e93b..55f4a89c85 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -865,6 +865,14 @@ class CrashpadClient { //! \param[in] attachment The path to the file to be added. void AddAttachment(const base::FilePath& attachment); + //! \brief Writes content to a handler-side attachment file. + bool WriteAttachment( + const base::FilePath& attachment, const std::string& data); + + //! \brief Appends content to a handler-side attachment file. + bool AppendAttachment( + const base::FilePath& attachment, const std::string& data); + //! \brief Removes a file from the list of files to be attached to the crash //! report. //! diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index 43844df03c..7d8abbc49a 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -440,6 +440,18 @@ class RequestCrashDumpHandler : public SignalHandler { client.AddAttachment(attachment); } + bool WriteAttachment(const base::FilePath& attachment, + const std::string& data) { + ExceptionHandlerClient client(sock_to_handler_.get(), true); + return client.WriteAttachment(attachment, data); + } + + bool AppendAttachment(const base::FilePath& attachment, + const std::string& data) { + ExceptionHandlerClient client(sock_to_handler_.get(), true); + return client.AppendAttachment(attachment, data); + } + void RemoveAttachment(const base::FilePath& attachment) { ExceptionHandlerClient client(sock_to_handler_.get(), true); client.RemoveAttachment(attachment); @@ -832,6 +844,18 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) { signal_handler->AddAttachment(attachment); } +bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, + const std::string& data) { + auto signal_handler = RequestCrashDumpHandler::Get(); + return signal_handler->WriteAttachment(attachment, data); +} + +bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, + const std::string& data) { + auto signal_handler = RequestCrashDumpHandler::Get(); + return signal_handler->AppendAttachment(attachment, data); +} + void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { auto signal_handler = RequestCrashDumpHandler::Get(); signal_handler->RemoveAttachment(attachment); diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index faf7f282df..4badecbc4d 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -637,6 +637,30 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) { attachment.value()); } +bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, + const std::string& data) { + std::string payload; + payload.push_back('\0'); + payload.append(attachment.value()); + payload.push_back('\0'); + payload.append(data); + return SendClientToServerMessage(exception_port_.get(), + ClientToServerMessage::kWriteAttachment, + payload); +} + +bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, + const std::string& data) { + std::string payload; + payload.push_back('\1'); + payload.append(attachment.value()); + payload.push_back('\0'); + payload.append(data); + return SendClientToServerMessage(exception_port_.get(), + ClientToServerMessage::kWriteAttachment, + payload); +} + void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { SendClientToServerMessage(exception_port_.get(), ClientToServerMessage::kRemoveAttachment, diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 15cbe93ddc..f00af7899b 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -1216,19 +1216,109 @@ void CrashpadClient::SetFirstChanceExceptionHandler( } void CrashpadClient::AddAttachment(const base::FilePath& attachment) { + const size_t path_length_bytes = + (attachment.value().length() + 1) * sizeof(wchar_t); + if (path_length_bytes > kMaxPathBytes) { + LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes"; + return; + } + + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kAddAttachmentV2; + message.attachment_v2.path_length_bytes = + static_cast(path_length_bytes); + + ServerToClientMessage response = {}; + SendPayloadToCrashHandlerServer(ipc_pipe_, + message, + attachment.value().c_str(), + static_cast(path_length_bytes), + &response); +} + +bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, + const std::string& data) { + const size_t path_length_bytes = + (attachment.value().length() + 1) * sizeof(wchar_t); + if (path_length_bytes > kMaxPathBytes || + data.size() > UINT32_MAX - path_length_bytes) { + LOG(ERROR) << "attachment content too large"; + return false; + } + + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kWriteAttachment; + message.attachment_write.path_length_bytes = + static_cast(path_length_bytes); + message.attachment_write.payload_length_bytes = + static_cast(data.size()); + message.attachment_write.operation = kAttachmentWriteReplace; + + std::string payload(path_length_bytes + data.size(), '\0'); + memcpy(&payload[0], attachment.value().c_str(), path_length_bytes); + if (!data.empty()) { + memcpy(&payload[path_length_bytes], data.data(), data.size()); + } + ServerToClientMessage response = {}; - SendAttachmentToCrashHandlerServer(ipc_pipe_, - ClientToServerMessage::kAddAttachmentV2, - attachment.value(), - &response); + return SendPayloadToCrashHandlerServer(ipc_pipe_, + message, + payload.data(), + static_cast(payload.size()), + &response); +} + +bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, + const std::string& data) { + const size_t path_length_bytes = + (attachment.value().length() + 1) * sizeof(wchar_t); + if (path_length_bytes > kMaxPathBytes || + data.size() > UINT32_MAX - path_length_bytes) { + LOG(ERROR) << "attachment content too large"; + return false; + } + + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kWriteAttachment; + message.attachment_write.path_length_bytes = + static_cast(path_length_bytes); + message.attachment_write.payload_length_bytes = + static_cast(data.size()); + message.attachment_write.operation = kAttachmentWriteAppend; + + std::string payload(path_length_bytes + data.size(), '\0'); + memcpy(&payload[0], attachment.value().c_str(), path_length_bytes); + if (!data.empty()) { + memcpy(&payload[path_length_bytes], data.data(), data.size()); + } + + ServerToClientMessage response = {}; + return SendPayloadToCrashHandlerServer(ipc_pipe_, + message, + payload.data(), + static_cast(payload.size()), + &response); } void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { + const size_t path_length_bytes = + (attachment.value().length() + 1) * sizeof(wchar_t); + if (path_length_bytes > kMaxPathBytes) { + LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes"; + return; + } + + ClientToServerMessage message = {}; + message.type = ClientToServerMessage::kRemoveAttachmentV2; + message.attachment_v2.path_length_bytes = + static_cast(path_length_bytes); + ServerToClientMessage response = {}; - SendAttachmentToCrashHandlerServer(ipc_pipe_, - ClientToServerMessage::kRemoveAttachmentV2, - attachment.value(), - &response); + SendPayloadToCrashHandlerServer(ipc_pipe_, + message, + attachment.value().c_str(), + static_cast(path_length_bytes), + &response); } void CrashpadClient::RequestRetry() { diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc index 11f0f47176..19c3ced455 100644 --- a/handler/linux/crash_report_exception_handler.cc +++ b/handler/linux/crash_report_exception_handler.cc @@ -26,6 +26,7 @@ #include "snapshot/sanitized/process_snapshot_sanitized.h" #include "util/file/file_helper.h" #include "util/file/file_reader.h" +#include "util/file/file_writer.h" #include "util/file/output_stream_file_writer.h" #include "util/linux/direct_ptrace_connection.h" #include "util/linux/ptrace_client.h" @@ -226,6 +227,31 @@ void CrashReportExceptionHandler::AddAttachment( attachments_.push_back(attachment); } +void CrashReportExceptionHandler::WriteAttachment( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + +void CrashReportExceptionHandler::AppendAttachment( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kReuseOrCreate, + FilePermissions::kOwnerOnly) || + writer.Seek(0, SEEK_END) < 0 || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + void CrashReportExceptionHandler::RemoveAttachment( const base::FilePath& attachment) { auto it = std::find(attachments_.begin(), attachments_.end(), attachment); diff --git a/handler/linux/crash_report_exception_handler.h b/handler/linux/crash_report_exception_handler.h index 269e00c4c7..b7034d6c8b 100644 --- a/handler/linux/crash_report_exception_handler.h +++ b/handler/linux/crash_report_exception_handler.h @@ -99,6 +99,10 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { UUID* local_report_id = nullptr) override; void AddAttachment(const base::FilePath& attachment) override; + void WriteAttachment( + const base::FilePath& attachment, const std::string& data) override; + void AppendAttachment( + const base::FilePath& attachment, const std::string& data) override; void RemoveAttachment(const base::FilePath& attachment) override; void RequestRetry() override; diff --git a/handler/linux/exception_handler_server.cc b/handler/linux/exception_handler_server.cc index 82a185624c..6c53c256f1 100644 --- a/handler/linux/exception_handler_server.cc +++ b/handler/linux/exception_handler_server.cc @@ -24,6 +24,7 @@ #include #include +#include #include "base/check_op.h" #include "base/compiler_specific.h" @@ -452,13 +453,31 @@ bool ExceptionHandlerServer::UninstallClientSocket(Event* event) { } bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) { - ExceptionHandlerProtocol::ClientToServerMessage message; + std::vector message_buffer( + sizeof(ExceptionHandlerProtocol::ClientToServerMessage) + PATH_MAX + + ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize); ucred creds; + ssize_t bytes_received = 0; if (!UnixCredentialSocket::RecvMsg( - event->fd.get(), &message, sizeof(message), &creds)) { + event->fd.get(), + message_buffer.data(), + message_buffer.size(), + &creds, + nullptr, + &bytes_received)) { + return false; + } + if (bytes_received < + static_cast( + sizeof(ExceptionHandlerProtocol::ClientToServerMessage))) { + LOG(ERROR) << "short client message"; return false; } + const auto& message = + *reinterpret_cast( + message_buffer.data()); + switch (message.type) { case ExceptionHandlerProtocol::ClientToServerMessage::kTypeCheckCredentials: return SendCredentials(event->fd.get()); @@ -491,6 +510,42 @@ bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) { } delegate_->RequestRetry(); return true; + + case ExceptionHandlerProtocol::ClientToServerMessage:: + kTypeWriteAttachment: { + const uint32_t path_size = message.attachment_write_info.path_size; + const uint32_t payload_size = + message.attachment_write_info.payload_size; + const auto operation = static_cast< + ExceptionHandlerProtocol::AttachmentWriteOperation>( + message.attachment_write_info.operation); + if (path_size == 0 || path_size > PATH_MAX || + payload_size > + ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize) { + LOG(ERROR) << "invalid attachment write message"; + return true; + } + const size_t expected_size = + sizeof(ExceptionHandlerProtocol::ClientToServerMessage) + path_size + + payload_size; + if (bytes_received != static_cast(expected_size)) { + LOG(ERROR) << "truncated attachment write message"; + return true; + } + + const char* path = message_buffer.data() + + sizeof( + ExceptionHandlerProtocol::ClientToServerMessage); + const char* payload = path + path_size; + if (operation == ExceptionHandlerProtocol::kAttachmentWriteAppend) { + delegate_->AppendAttachment( + base::FilePath(path), std::string(payload, payload_size)); + } else { + delegate_->WriteAttachment( + base::FilePath(path), std::string(payload, payload_size)); + } + return true; + } } DCHECK(false); diff --git a/handler/linux/exception_handler_server.h b/handler/linux/exception_handler_server.h index 09aeab461b..c021a257a0 100644 --- a/handler/linux/exception_handler_server.h +++ b/handler/linux/exception_handler_server.h @@ -20,6 +20,7 @@ #include #include +#include #include #include "util/file/file_io.h" @@ -114,6 +115,14 @@ class ExceptionHandlerServer { //! \brief Called to add an attachment to the crash report. virtual void AddAttachment(const base::FilePath& attachment) = 0; + //! \brief Called to write an attachment's contents. + virtual void WriteAttachment( + const base::FilePath& attachment, const std::string& data) = 0; + + //! \brief Called to append to an attachment's contents. + virtual void AppendAttachment( + const base::FilePath& attachment, const std::string& data) = 0; + //! \brief Called to remove an attachment from the crash report. virtual void RemoveAttachment(const base::FilePath& attachment) = 0; diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index a6341abc8a..cdb240b445 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -336,6 +336,31 @@ void CrashReportExceptionHandler::AddAttachment( attachments_.push_back(attachment); } +void CrashReportExceptionHandler::WriteAttachment( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + +void CrashReportExceptionHandler::AppendAttachment( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kReuseOrCreate, + FilePermissions::kOwnerOnly) || + writer.Seek(0, SEEK_END) < 0 || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + void CrashReportExceptionHandler::RemoveAttachment( const base::FilePath& attachment) { auto it = std::find(attachments_.begin(), attachments_.end(), attachment); diff --git a/handler/mac/crash_report_exception_handler.h b/handler/mac/crash_report_exception_handler.h index 2fe4ba0393..5af5ccefbd 100644 --- a/handler/mac/crash_report_exception_handler.h +++ b/handler/mac/crash_report_exception_handler.h @@ -99,6 +99,10 @@ class CrashReportExceptionHandler final // ExceptionHandlerServer::Delegate: void RequestRetry() override; void AddAttachment(const base::FilePath& attachment) override; + void WriteAttachment( + const base::FilePath& attachment, const std::string& data) override; + void AppendAttachment( + const base::FilePath& attachment, const std::string& data) override; void RemoveAttachment(const base::FilePath& attachment) override; private: diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index bdcae577ed..3ab47bf142 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -65,6 +65,29 @@ class ClientToServerMessageServer : public MachMessageServer::Interface { case ClientToServerMessage::kAddAttachment: delegate_->AddAttachment(base::FilePath(message->Payload())); break; + case ClientToServerMessage::kWriteAttachment: { + std::string payload = message->Payload(); + if (payload.empty()) { + LOG(ERROR) << "malformed attachment write message"; + break; + } + bool should_append = payload[0] != '\0'; + size_t path_end = payload.find('\0', 1); + if (path_end == std::string::npos) { + LOG(ERROR) << "malformed attachment write message"; + break; + } + if (should_append) { + delegate_->AppendAttachment( + base::FilePath(payload.substr(1, path_end - 1)), + payload.substr(path_end + 1)); + } else { + delegate_->WriteAttachment( + base::FilePath(payload.substr(1, path_end - 1)), + payload.substr(path_end + 1)); + } + break; + } case ClientToServerMessage::kRemoveAttachment: delegate_->RemoveAttachment(base::FilePath(message->Payload())); break; diff --git a/handler/mac/exception_handler_server.h b/handler/mac/exception_handler_server.h index 5f7bd8337b..472e81bf96 100644 --- a/handler/mac/exception_handler_server.h +++ b/handler/mac/exception_handler_server.h @@ -18,6 +18,8 @@ #include #include +#include + #include "base/apple/scoped_mach_port.h" #include "base/files/file_path.h" #include "util/mach/exc_server_variants.h" @@ -42,6 +44,16 @@ class ExceptionHandlerServer { //! the list of files attached to crash reports. virtual void AddAttachment(const base::FilePath& attachment) = 0; + //! \brief Called when the server has received a request to write an + //! attachment's contents. + virtual void WriteAttachment( + const base::FilePath& attachment, const std::string& data) = 0; + + //! \brief Called when the server has received a request to append to an + //! attachment's contents. + virtual void AppendAttachment( + const base::FilePath& attachment, const std::string& data) = 0; + //! \brief Called when the server has received a request to remove a file //! from the list of files attached to crash reports. virtual void RemoveAttachment(const base::FilePath& attachment) = 0; diff --git a/handler/win/crash_report_exception_handler.cc b/handler/win/crash_report_exception_handler.cc index 8e06556b05..b562d02f66 100644 --- a/handler/win/crash_report_exception_handler.cc +++ b/handler/win/crash_report_exception_handler.cc @@ -207,6 +207,31 @@ void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentAdded( attachments_.push_back(attachment); } +void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentWritten( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + +void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentAppended( + const base::FilePath& attachment, const std::string& data) { + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kReuseOrCreate, + FilePermissions::kOwnerOnly) || + writer.Seek(0, SEEK_END) < 0 || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return; + } +} + void CrashReportExceptionHandler::ExceptionHandlerServerAttachmentRemoved( const base::FilePath& attachment) { auto it = std::find(attachments_.begin(), attachments_.end(), attachment); diff --git a/handler/win/crash_report_exception_handler.h b/handler/win/crash_report_exception_handler.h index 9fe36b00dd..a0443481c9 100644 --- a/handler/win/crash_report_exception_handler.h +++ b/handler/win/crash_report_exception_handler.h @@ -85,6 +85,10 @@ class CrashReportExceptionHandler final WinVMAddress debug_critical_section_address) override; void ExceptionHandlerServerAttachmentAdded( const base::FilePath& attachment) override; + void ExceptionHandlerServerAttachmentWritten( + const base::FilePath& attachment, const std::string& data) override; + void ExceptionHandlerServerAttachmentAppended( + const base::FilePath& attachment, const std::string& data) override; void ExceptionHandlerServerAttachmentRemoved( const base::FilePath& attachment) override; void ExceptionHandlerServerRetryRequested() override; diff --git a/util/linux/exception_handler_client.cc b/util/linux/exception_handler_client.cc index 306a932e38..3398fb3eee 100644 --- a/util/linux/exception_handler_client.cc +++ b/util/linux/exception_handler_client.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -26,6 +27,7 @@ #include "build/build_config.h" #include "third_party/lss/lss.h" #include "util/file/file_io.h" +#include "util/file/file_writer.h" #include "util/linux/ptrace_broker.h" #include "util/linux/socket.h" #include "util/misc/from_pointer_cast.h" @@ -62,6 +64,71 @@ class ScopedSigprocmaskRestore { bool mask_is_set_; }; +bool WriteAttachmentFile( + const base::FilePath& attachment, + const std::string& data, + ExceptionHandlerProtocol::AttachmentWriteOperation operation) { + FileWriter writer; + if (!writer.Open(attachment, + operation == ExceptionHandlerProtocol::kAttachmentWriteAppend + ? FileWriteMode::kReuseOrCreate + : FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + (operation == ExceptionHandlerProtocol::kAttachmentWriteAppend && + writer.Seek(0, SEEK_END) < 0) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return false; + } + return true; +} + +bool SendAttachmentWrite( + int server_sock, + const base::FilePath& attachment, + const std::string& data, + ExceptionHandlerProtocol::AttachmentWriteOperation operation) { + if (attachment.value().size() + 1 > PATH_MAX) { + LOG(ERROR) << "attachment path too long: " << attachment.value().size() + << " bytes"; + return false; + } + if (data.size() > + ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize) { + return WriteAttachmentFile(attachment, data, operation); + } + + ExceptionHandlerProtocol::ClientToServerMessage message; + message.type = + ExceptionHandlerProtocol::ClientToServerMessage::kTypeWriteAttachment; + message.attachment_write_info.path_size = + static_cast(attachment.value().size() + 1); + message.attachment_write_info.payload_size = + static_cast(data.size()); + message.attachment_write_info.operation = operation; + + std::string payload(sizeof(message) + attachment.value().size() + 1 + + data.size(), + '\0'); + memcpy(&payload[0], &message, sizeof(message)); + memcpy(&payload[sizeof(message)], attachment.value().c_str(), + attachment.value().size() + 1); + if (!data.empty()) { + memcpy(&payload[sizeof(message) + attachment.value().size() + 1], + data.data(), + data.size()); + } + int result = + UnixCredentialSocket::SendMsg(server_sock, payload.data(), payload.size()); + if (result == 0) { + return true; + } + if (result == EMSGSIZE || result == ENOBUFS) { + return WriteAttachmentFile(attachment, data, operation); + } + return false; +} + } // namespace ExceptionHandlerClient::ExceptionHandlerClient(int sock, bool multiple_clients) @@ -234,6 +301,24 @@ void ExceptionHandlerClient::AddAttachment(const base::FilePath& attachment) { UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message)); } +bool ExceptionHandlerClient::WriteAttachment(const base::FilePath& attachment, + const std::string& data) { + return SendAttachmentWrite( + server_sock_, + attachment, + data, + ExceptionHandlerProtocol::kAttachmentWriteReplace); +} + +bool ExceptionHandlerClient::AppendAttachment(const base::FilePath& attachment, + const std::string& data) { + return SendAttachmentWrite( + server_sock_, + attachment, + data, + ExceptionHandlerProtocol::kAttachmentWriteAppend); +} + void ExceptionHandlerClient::RemoveAttachment( const base::FilePath& attachment) { ExceptionHandlerProtocol::ClientToServerMessage message; diff --git a/util/linux/exception_handler_client.h b/util/linux/exception_handler_client.h index a7e4c66dfb..6b74893a4f 100644 --- a/util/linux/exception_handler_client.h +++ b/util/linux/exception_handler_client.h @@ -18,6 +18,8 @@ #include #include +#include + #include "util/linux/exception_handler_protocol.h" namespace crashpad { @@ -71,6 +73,14 @@ class ExceptionHandlerClient { //! \brief Adds an attachment to the crash report. void AddAttachment(const base::FilePath& attachment); + //! \brief Requests that the handler write an attachment's contents. + bool WriteAttachment( + const base::FilePath& attachment, const std::string& data); + + //! \brief Requests that the handler append to an attachment's contents. + bool AppendAttachment( + const base::FilePath& attachment, const std::string& data); + //! \brief Removes an attachment from the crash report. void RemoveAttachment(const base::FilePath& attachment); diff --git a/util/linux/exception_handler_protocol.h b/util/linux/exception_handler_protocol.h index 0b6a642f00..afbc1f8178 100644 --- a/util/linux/exception_handler_protocol.h +++ b/util/linux/exception_handler_protocol.h @@ -64,6 +64,19 @@ class ExceptionHandlerProtocol { char path[PATH_MAX]; }; + static constexpr uint32_t kMaxAttachmentWritePayloadSize = 192 * 1024; + + enum AttachmentWriteOperation : uint32_t { + kAttachmentWriteReplace, + kAttachmentWriteAppend, + }; + + struct AttachmentWriteInformation { + uint32_t path_size; + uint32_t payload_size; + uint32_t operation; + }; + //! \brief The signal used to indicate a crash dump is complete. //! //! When multiple clients share a single socket connection with the handler, @@ -96,6 +109,9 @@ class ExceptionHandlerProtocol { //! \brief Request that the server retry pending report uploads. kTypeRequestRetry, + + //! \brief Request that the server write an attachment's contents. + kTypeWriteAttachment, }; Type type; @@ -109,6 +125,9 @@ class ExceptionHandlerProtocol { //! \brief Valid for type == kAddAttachment || type == kRemoveAttachment AttachmentInformation attachment_info; + + //! \brief Valid for type == kTypeWriteAttachment. + AttachmentWriteInformation attachment_write_info; }; }; diff --git a/util/linux/socket.cc b/util/linux/socket.cc index bad5056f9f..6641bb5205 100644 --- a/util/linux/socket.cc +++ b/util/linux/socket.cc @@ -101,7 +101,8 @@ bool UnixCredentialSocket::RecvMsg(int fd, void* buf, size_t buf_size, ucred* creds, - std::vector* fds) { + std::vector* fds, + ssize_t* bytes_received) { iovec iov; iov.iov_base = buf; iov.iov_len = buf_size; @@ -120,6 +121,9 @@ bool UnixCredentialSocket::RecvMsg(int fd, PLOG(ERROR) << "recvmsg"; return false; } + if (bytes_received) { + *bytes_received = res; + } ucred* local_creds = nullptr; std::vector local_fds; @@ -178,7 +182,7 @@ bool UnixCredentialSocket::RecvMsg(int fd, return false; } - if (static_cast(res) != buf_size) { + if (!bytes_received && static_cast(res) != buf_size) { LOG(ERROR) << "incorrect payload size " << res; return false; } diff --git a/util/linux/socket.h b/util/linux/socket.h index 4bde25c814..3545c0e99a 100644 --- a/util/linux/socket.h +++ b/util/linux/socket.h @@ -88,7 +88,8 @@ class UnixCredentialSocket { void* buf, size_t buf_size, ucred* creds, - std::vector* fds = nullptr); + std::vector* fds = nullptr, + ssize_t* bytes_received = nullptr); }; } // namespace crashpad diff --git a/util/mach/exception_handler_protocol.h b/util/mach/exception_handler_protocol.h index cdd3af1d1d..7fe9d2df85 100644 --- a/util/mach/exception_handler_protocol.h +++ b/util/mach/exception_handler_protocol.h @@ -49,6 +49,10 @@ struct ClientToServerMessage { //! \brief Remove a file from the list of files attached to crash reports. //! The payload contains the attachment path. kRemoveAttachment = 3, + + //! \brief Write an attachment's contents. The payload contains a + //! null-terminated attachment path followed by the attachment content. + kWriteAttachment = 4, }; mach_msg_header_t header; diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 2a19023c9d..07b55660bf 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -528,6 +528,59 @@ static void HandleRemoveAttachmentV2( LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); } +static void HandleWriteAttachment( + const internal::PipeServiceContext& service_context, + const ClientToServerMessage& message) { + const uint32_t path_length_bytes = + message.attachment_write.path_length_bytes; + const uint32_t payload_length_bytes = + message.attachment_write.payload_length_bytes; + const auto operation = + static_cast(message.attachment_write.operation); + + if (path_length_bytes == 0 || path_length_bytes > kMaxPathBytes || + path_length_bytes % sizeof(wchar_t) != 0) { + LOG(ERROR) << "Invalid path length: " << path_length_bytes; + return; + } + if (payload_length_bytes > UINT32_MAX - path_length_bytes) { + LOG(ERROR) << "Invalid attachment write length"; + return; + } + + const uint32_t request_payload_length_bytes = + path_length_bytes + payload_length_bytes; + std::string request_payload(request_payload_length_bytes, '\0'); + if (!LoggingReadFileExactly( + service_context.pipe(), + &request_payload[0], + request_payload_length_bytes)) { + LOG(ERROR) << "Failed to read attachment write"; + return; + } + + const size_t path_length = path_length_bytes / sizeof(wchar_t) - 1; + std::wstring path(path_length, L'\0'); + if (path_length > 0) { + memcpy(&path[0], + request_payload.data(), + path_length_bytes - sizeof(wchar_t)); + } + + std::string payload( + request_payload.data() + path_length_bytes, payload_length_bytes); + + ServerToClientMessage response = {}; + if (operation == kAttachmentWriteAppend) { + service_context.delegate()->ExceptionHandlerServerAttachmentAppended( + base::FilePath(path), payload); + } else { + service_context.delegate()->ExceptionHandlerServerAttachmentWritten( + base::FilePath(path), payload); + } + LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); +} + // This function must be called with service_context.pipe() already connected to // a client pipe. It exchanges data with the client and adds a ClientData record // to service_context->clients(). @@ -611,6 +664,11 @@ bool ExceptionHandlerServer::ServiceClientConnection( return false; } + case ClientToServerMessage::kWriteAttachment: { + HandleWriteAttachment(service_context, message); + return false; + } + case ClientToServerMessage::kRequestRetry: { if (!RuntimeMessageOriginIsOwner(service_context)) { return false; diff --git a/util/win/exception_handler_server.h b/util/win/exception_handler_server.h index ecea045bd9..e56b2723aa 100644 --- a/util/win/exception_handler_server.h +++ b/util/win/exception_handler_server.h @@ -65,6 +65,16 @@ class ExceptionHandlerServer { virtual void ExceptionHandlerServerAttachmentAdded( const base::FilePath& attachment) = 0; + //! \brief Called when the server has received a request to write an + //! attachment's contents. + virtual void ExceptionHandlerServerAttachmentWritten( + const base::FilePath& attachment, const std::string& data) = 0; + + //! \brief Called when the server has received a request to append to an + //! attachment's contents. + virtual void ExceptionHandlerServerAttachmentAppended( + const base::FilePath& attachment, const std::string& data) = 0; + //! \brief Called when the server has received a request to remove an //! attachment. //! diff --git a/util/win/registration_protocol_win.cc b/util/win/registration_protocol_win.cc index 8b0c445a41..503c00fef3 100644 --- a/util/win/registration_protocol_win.cc +++ b/util/win/registration_protocol_win.cc @@ -141,30 +141,12 @@ bool SendToCrashHandlerServer(const std::wstring& pipe_name, } } -bool SendAttachmentToCrashHandlerServer( +bool SendPayloadToCrashHandlerServer( const std::wstring& pipe_name, - ClientToServerMessage::Type message_type, - const std::wstring& path, + const ClientToServerMessage& message, + const void* payload, + uint32_t payload_size, ServerToClientMessage* response) { - if (message_type != ClientToServerMessage::kAddAttachmentV2 && - message_type != ClientToServerMessage::kRemoveAttachmentV2) { - LOG(ERROR) << "Invalid message type for attachment: " << message_type; - return false; - } - - const size_t path_length_bytes = (path.length() + 1) * sizeof(wchar_t); - - if (path_length_bytes > kMaxPathBytes) { - LOG(ERROR) << "Path too long: " << path_length_bytes << " bytes"; - return false; - } - - // Build the message header. - ClientToServerMessage message = {}; - message.type = message_type; - message.attachment_v2.path_length_bytes = - static_cast(path_length_bytes); - // Retry CreateFile() in a loop (follows the logic in // SendToCrashHandlerServer). for (;;) { @@ -201,9 +183,9 @@ bool SendAttachmentToCrashHandlerServer( return false; } - if (!WriteFile( - pipe.get(), path.c_str(), static_cast(path_length_bytes))) { - PLOG(ERROR) << "WriteFile (path)"; + if (payload_size > 0 && + !WriteFile(pipe.get(), payload, static_cast(payload_size))) { + PLOG(ERROR) << "WriteFile (payload)"; return false; } diff --git a/util/win/registration_protocol_win.h b/util/win/registration_protocol_win.h index 5bf569ac02..90de5ea2bf 100644 --- a/util/win/registration_protocol_win.h +++ b/util/win/registration_protocol_win.h @@ -36,22 +36,24 @@ bool SendToCrashHandlerServer(const std::wstring& pipe_name, const ClientToServerMessage& message, ServerToClientMessage* response); -//! \brief Connect over the given \a pipe_name, passing a variable-length -//! attachment message to the server. +//! \brief Connect over the given \a pipe_name, passing a message with a +//! variable-length payload to the server. //! -//! This is used for kAddAttachmentV2 and kRemoveAttachmentV2 message types -//! which support paths longer than MAX_PATH. +//! This is used for messages whose `ClientToServerMessage` header is followed +//! by additional payload bytes. //! //! \param[in] pipe_name The name of the pipe to connect to. -//! \param[in] message_type Either kAddAttachmentV2 or kRemoveAttachmentV2. -//! \param[in] path The attachment path to send. +//! \param[in] message The message header to send. +//! \param[in] payload The payload bytes to send after \a message. +//! \param[in] payload_size The size of \a payload. //! \param[out] response The server's response. //! //! \return `true` on success, `false` on failure with a message logged. -bool SendAttachmentToCrashHandlerServer( +bool SendPayloadToCrashHandlerServer( const std::wstring& pipe_name, - ClientToServerMessage::Type message_type, - const std::wstring& path, + const ClientToServerMessage& message, + const void* payload, + uint32_t payload_size, ServerToClientMessage* response); //! \brief Wraps CreateNamedPipe() to create a single named pipe instance. diff --git a/util/win/registration_protocol_win_structs.h b/util/win/registration_protocol_win_structs.h index 61f95f7b42..e565bbdea5 100644 --- a/util/win/registration_protocol_win_structs.h +++ b/util/win/registration_protocol_win_structs.h @@ -132,6 +132,28 @@ struct AttachmentRequestV2 { uint32_t path_length_bytes; }; +enum AttachmentWriteOperation : uint32_t { + kAttachmentWriteReplace, + kAttachmentWriteAppend, +}; + +//! \brief A variable-length attachment write request header. +//! +//! For kWriteAttachment, the message consists of a ClientToServerMessage +//! with this header in the union, followed by path_length_bytes of wchar_t data +//! containing the null-terminated path and payload_length_bytes of attachment +//! content. +struct AttachmentWriteRequest { + //! \brief Length of the path in bytes, including null terminator. + uint32_t path_length_bytes; + + //! \brief Length of the attachment content in bytes. + uint32_t payload_length_bytes; + + //! \brief The write operation to apply to the attachment content. + uint32_t operation; +}; + //! follow the maximum path length documented here: //! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation constexpr uint32_t kMaxPathBytes = 32768 * sizeof(wchar_t); @@ -167,6 +189,9 @@ struct ClientToServerMessage { //! \brief Requests that the server retry pending report uploads. No //! additional payload. kRequestRetry, + + //! \brief For AttachmentWriteRequest. + kWriteAttachment, } type; union { @@ -174,6 +199,7 @@ struct ClientToServerMessage { ShutdownRequest shutdown; AttachmentRequest attachment; AttachmentRequestV2 attachment_v2; + AttachmentWriteRequest attachment_write; }; }; From 4d2862c70dae7b84a97cbbc98fc274ccd4373515 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 6 Jul 2026 14:08:08 +0200 Subject: [PATCH 2/8] Drop IPC on Unix - simple file operations are fast enough --- client/crashpad_client.h | 4 +- client/crashpad_client_linux.cc | 37 ++++---- client/crashpad_client_mac.cc | 39 +++++---- .../linux/crash_report_exception_handler.cc | 26 ------ .../linux/crash_report_exception_handler.h | 4 - handler/linux/exception_handler_server.cc | 59 +------------ handler/linux/exception_handler_server.h | 9 -- handler/mac/crash_report_exception_handler.cc | 25 ------ handler/mac/crash_report_exception_handler.h | 4 - handler/mac/exception_handler_server.cc | 23 ----- handler/mac/exception_handler_server.h | 12 --- util/linux/exception_handler_client.cc | 85 ------------------- util/linux/exception_handler_client.h | 10 --- util/linux/exception_handler_protocol.h | 19 ----- util/linux/socket.cc | 8 +- util/linux/socket.h | 3 +- util/mach/exception_handler_protocol.h | 4 - 17 files changed, 50 insertions(+), 321 deletions(-) diff --git a/client/crashpad_client.h b/client/crashpad_client.h index 55f4a89c85..1bd8bd66de 100644 --- a/client/crashpad_client.h +++ b/client/crashpad_client.h @@ -865,11 +865,11 @@ class CrashpadClient { //! \param[in] attachment The path to the file to be added. void AddAttachment(const base::FilePath& attachment); - //! \brief Writes content to a handler-side attachment file. + //! \brief Writes content to an attachment file. bool WriteAttachment( const base::FilePath& attachment, const std::string& data); - //! \brief Appends content to a handler-side attachment file. + //! \brief Appends content to an attachment file. bool AppendAttachment( const base::FilePath& attachment, const std::string& data); diff --git a/client/crashpad_client_linux.cc b/client/crashpad_client_linux.cc index 7d8abbc49a..ff0d67fb8b 100644 --- a/client/crashpad_client_linux.cc +++ b/client/crashpad_client_linux.cc @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -38,6 +39,7 @@ #include "third_party/lss/lss.h" #include "util/file/file_io.h" #include "util/file/filesystem.h" +#include "util/file/file_writer.h" #include "util/linux/exception_handler_client.h" #include "util/linux/exception_information.h" #include "util/linux/scoped_pr_set_dumpable.h" @@ -440,18 +442,6 @@ class RequestCrashDumpHandler : public SignalHandler { client.AddAttachment(attachment); } - bool WriteAttachment(const base::FilePath& attachment, - const std::string& data) { - ExceptionHandlerClient client(sock_to_handler_.get(), true); - return client.WriteAttachment(attachment, data); - } - - bool AppendAttachment(const base::FilePath& attachment, - const std::string& data) { - ExceptionHandlerClient client(sock_to_handler_.get(), true); - return client.AppendAttachment(attachment, data); - } - void RemoveAttachment(const base::FilePath& attachment) { ExceptionHandlerClient client(sock_to_handler_.get(), true); client.RemoveAttachment(attachment); @@ -846,14 +836,29 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) { bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, const std::string& data) { - auto signal_handler = RequestCrashDumpHandler::Get(); - return signal_handler->WriteAttachment(attachment, data); + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return false; + } + return true; } bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, const std::string& data) { - auto signal_handler = RequestCrashDumpHandler::Get(); - return signal_handler->AppendAttachment(attachment, data); + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kReuseOrCreate, + FilePermissions::kOwnerOnly) || + writer.Seek(0, SEEK_END) < 0 || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return false; + } + return true; } void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index 4badecbc4d..e1e7469cc2 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -19,7 +19,8 @@ #include #include #include -#include +#include +#include #include #include @@ -29,6 +30,7 @@ #include "base/check_op.h" #include "base/logging.h" #include "base/strings/stringprintf.h" +#include "util/file/file_writer.h" #include "util/mac/mac_util.h" #include "util/mach/bootstrap.h" #include "util/mach/child_port_handshake.h" @@ -639,26 +641,29 @@ void CrashpadClient::AddAttachment(const base::FilePath& attachment) { bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, const std::string& data) { - std::string payload; - payload.push_back('\0'); - payload.append(attachment.value()); - payload.push_back('\0'); - payload.append(data); - return SendClientToServerMessage(exception_port_.get(), - ClientToServerMessage::kWriteAttachment, - payload); + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kTruncateOrCreate, + FilePermissions::kOwnerOnly) || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return false; + } + return true; } bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, const std::string& data) { - std::string payload; - payload.push_back('\1'); - payload.append(attachment.value()); - payload.push_back('\0'); - payload.append(data); - return SendClientToServerMessage(exception_port_.get(), - ClientToServerMessage::kWriteAttachment, - payload); + FileWriter writer; + if (!writer.Open(attachment, + FileWriteMode::kReuseOrCreate, + FilePermissions::kOwnerOnly) || + writer.Seek(0, SEEK_END) < 0 || + !writer.Write(data.data(), data.size())) { + LOG(ERROR) << "failed to write attachment " << attachment; + return false; + } + return true; } void CrashpadClient::RemoveAttachment(const base::FilePath& attachment) { diff --git a/handler/linux/crash_report_exception_handler.cc b/handler/linux/crash_report_exception_handler.cc index 19c3ced455..11f0f47176 100644 --- a/handler/linux/crash_report_exception_handler.cc +++ b/handler/linux/crash_report_exception_handler.cc @@ -26,7 +26,6 @@ #include "snapshot/sanitized/process_snapshot_sanitized.h" #include "util/file/file_helper.h" #include "util/file/file_reader.h" -#include "util/file/file_writer.h" #include "util/file/output_stream_file_writer.h" #include "util/linux/direct_ptrace_connection.h" #include "util/linux/ptrace_client.h" @@ -227,31 +226,6 @@ void CrashReportExceptionHandler::AddAttachment( attachments_.push_back(attachment); } -void CrashReportExceptionHandler::WriteAttachment( - const base::FilePath& attachment, const std::string& data) { - FileWriter writer; - if (!writer.Open(attachment, - FileWriteMode::kTruncateOrCreate, - FilePermissions::kOwnerOnly) || - !writer.Write(data.data(), data.size())) { - LOG(ERROR) << "failed to write attachment " << attachment; - return; - } -} - -void CrashReportExceptionHandler::AppendAttachment( - const base::FilePath& attachment, const std::string& data) { - FileWriter writer; - if (!writer.Open(attachment, - FileWriteMode::kReuseOrCreate, - FilePermissions::kOwnerOnly) || - writer.Seek(0, SEEK_END) < 0 || - !writer.Write(data.data(), data.size())) { - LOG(ERROR) << "failed to write attachment " << attachment; - return; - } -} - void CrashReportExceptionHandler::RemoveAttachment( const base::FilePath& attachment) { auto it = std::find(attachments_.begin(), attachments_.end(), attachment); diff --git a/handler/linux/crash_report_exception_handler.h b/handler/linux/crash_report_exception_handler.h index b7034d6c8b..269e00c4c7 100644 --- a/handler/linux/crash_report_exception_handler.h +++ b/handler/linux/crash_report_exception_handler.h @@ -99,10 +99,6 @@ class CrashReportExceptionHandler : public ExceptionHandlerServer::Delegate { UUID* local_report_id = nullptr) override; void AddAttachment(const base::FilePath& attachment) override; - void WriteAttachment( - const base::FilePath& attachment, const std::string& data) override; - void AppendAttachment( - const base::FilePath& attachment, const std::string& data) override; void RemoveAttachment(const base::FilePath& attachment) override; void RequestRetry() override; diff --git a/handler/linux/exception_handler_server.cc b/handler/linux/exception_handler_server.cc index 6c53c256f1..82a185624c 100644 --- a/handler/linux/exception_handler_server.cc +++ b/handler/linux/exception_handler_server.cc @@ -24,7 +24,6 @@ #include #include -#include #include "base/check_op.h" #include "base/compiler_specific.h" @@ -453,31 +452,13 @@ bool ExceptionHandlerServer::UninstallClientSocket(Event* event) { } bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) { - std::vector message_buffer( - sizeof(ExceptionHandlerProtocol::ClientToServerMessage) + PATH_MAX + - ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize); + ExceptionHandlerProtocol::ClientToServerMessage message; ucred creds; - ssize_t bytes_received = 0; if (!UnixCredentialSocket::RecvMsg( - event->fd.get(), - message_buffer.data(), - message_buffer.size(), - &creds, - nullptr, - &bytes_received)) { - return false; - } - if (bytes_received < - static_cast( - sizeof(ExceptionHandlerProtocol::ClientToServerMessage))) { - LOG(ERROR) << "short client message"; + event->fd.get(), &message, sizeof(message), &creds)) { return false; } - const auto& message = - *reinterpret_cast( - message_buffer.data()); - switch (message.type) { case ExceptionHandlerProtocol::ClientToServerMessage::kTypeCheckCredentials: return SendCredentials(event->fd.get()); @@ -510,42 +491,6 @@ bool ExceptionHandlerServer::ReceiveClientMessage(Event* event) { } delegate_->RequestRetry(); return true; - - case ExceptionHandlerProtocol::ClientToServerMessage:: - kTypeWriteAttachment: { - const uint32_t path_size = message.attachment_write_info.path_size; - const uint32_t payload_size = - message.attachment_write_info.payload_size; - const auto operation = static_cast< - ExceptionHandlerProtocol::AttachmentWriteOperation>( - message.attachment_write_info.operation); - if (path_size == 0 || path_size > PATH_MAX || - payload_size > - ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize) { - LOG(ERROR) << "invalid attachment write message"; - return true; - } - const size_t expected_size = - sizeof(ExceptionHandlerProtocol::ClientToServerMessage) + path_size + - payload_size; - if (bytes_received != static_cast(expected_size)) { - LOG(ERROR) << "truncated attachment write message"; - return true; - } - - const char* path = message_buffer.data() + - sizeof( - ExceptionHandlerProtocol::ClientToServerMessage); - const char* payload = path + path_size; - if (operation == ExceptionHandlerProtocol::kAttachmentWriteAppend) { - delegate_->AppendAttachment( - base::FilePath(path), std::string(payload, payload_size)); - } else { - delegate_->WriteAttachment( - base::FilePath(path), std::string(payload, payload_size)); - } - return true; - } } DCHECK(false); diff --git a/handler/linux/exception_handler_server.h b/handler/linux/exception_handler_server.h index c021a257a0..09aeab461b 100644 --- a/handler/linux/exception_handler_server.h +++ b/handler/linux/exception_handler_server.h @@ -20,7 +20,6 @@ #include #include -#include #include #include "util/file/file_io.h" @@ -115,14 +114,6 @@ class ExceptionHandlerServer { //! \brief Called to add an attachment to the crash report. virtual void AddAttachment(const base::FilePath& attachment) = 0; - //! \brief Called to write an attachment's contents. - virtual void WriteAttachment( - const base::FilePath& attachment, const std::string& data) = 0; - - //! \brief Called to append to an attachment's contents. - virtual void AppendAttachment( - const base::FilePath& attachment, const std::string& data) = 0; - //! \brief Called to remove an attachment from the crash report. virtual void RemoveAttachment(const base::FilePath& attachment) = 0; diff --git a/handler/mac/crash_report_exception_handler.cc b/handler/mac/crash_report_exception_handler.cc index cdb240b445..a6341abc8a 100644 --- a/handler/mac/crash_report_exception_handler.cc +++ b/handler/mac/crash_report_exception_handler.cc @@ -336,31 +336,6 @@ void CrashReportExceptionHandler::AddAttachment( attachments_.push_back(attachment); } -void CrashReportExceptionHandler::WriteAttachment( - const base::FilePath& attachment, const std::string& data) { - FileWriter writer; - if (!writer.Open(attachment, - FileWriteMode::kTruncateOrCreate, - FilePermissions::kOwnerOnly) || - !writer.Write(data.data(), data.size())) { - LOG(ERROR) << "failed to write attachment " << attachment; - return; - } -} - -void CrashReportExceptionHandler::AppendAttachment( - const base::FilePath& attachment, const std::string& data) { - FileWriter writer; - if (!writer.Open(attachment, - FileWriteMode::kReuseOrCreate, - FilePermissions::kOwnerOnly) || - writer.Seek(0, SEEK_END) < 0 || - !writer.Write(data.data(), data.size())) { - LOG(ERROR) << "failed to write attachment " << attachment; - return; - } -} - void CrashReportExceptionHandler::RemoveAttachment( const base::FilePath& attachment) { auto it = std::find(attachments_.begin(), attachments_.end(), attachment); diff --git a/handler/mac/crash_report_exception_handler.h b/handler/mac/crash_report_exception_handler.h index 5af5ccefbd..2fe4ba0393 100644 --- a/handler/mac/crash_report_exception_handler.h +++ b/handler/mac/crash_report_exception_handler.h @@ -99,10 +99,6 @@ class CrashReportExceptionHandler final // ExceptionHandlerServer::Delegate: void RequestRetry() override; void AddAttachment(const base::FilePath& attachment) override; - void WriteAttachment( - const base::FilePath& attachment, const std::string& data) override; - void AppendAttachment( - const base::FilePath& attachment, const std::string& data) override; void RemoveAttachment(const base::FilePath& attachment) override; private: diff --git a/handler/mac/exception_handler_server.cc b/handler/mac/exception_handler_server.cc index 3ab47bf142..bdcae577ed 100644 --- a/handler/mac/exception_handler_server.cc +++ b/handler/mac/exception_handler_server.cc @@ -65,29 +65,6 @@ class ClientToServerMessageServer : public MachMessageServer::Interface { case ClientToServerMessage::kAddAttachment: delegate_->AddAttachment(base::FilePath(message->Payload())); break; - case ClientToServerMessage::kWriteAttachment: { - std::string payload = message->Payload(); - if (payload.empty()) { - LOG(ERROR) << "malformed attachment write message"; - break; - } - bool should_append = payload[0] != '\0'; - size_t path_end = payload.find('\0', 1); - if (path_end == std::string::npos) { - LOG(ERROR) << "malformed attachment write message"; - break; - } - if (should_append) { - delegate_->AppendAttachment( - base::FilePath(payload.substr(1, path_end - 1)), - payload.substr(path_end + 1)); - } else { - delegate_->WriteAttachment( - base::FilePath(payload.substr(1, path_end - 1)), - payload.substr(path_end + 1)); - } - break; - } case ClientToServerMessage::kRemoveAttachment: delegate_->RemoveAttachment(base::FilePath(message->Payload())); break; diff --git a/handler/mac/exception_handler_server.h b/handler/mac/exception_handler_server.h index 472e81bf96..5f7bd8337b 100644 --- a/handler/mac/exception_handler_server.h +++ b/handler/mac/exception_handler_server.h @@ -18,8 +18,6 @@ #include #include -#include - #include "base/apple/scoped_mach_port.h" #include "base/files/file_path.h" #include "util/mach/exc_server_variants.h" @@ -44,16 +42,6 @@ class ExceptionHandlerServer { //! the list of files attached to crash reports. virtual void AddAttachment(const base::FilePath& attachment) = 0; - //! \brief Called when the server has received a request to write an - //! attachment's contents. - virtual void WriteAttachment( - const base::FilePath& attachment, const std::string& data) = 0; - - //! \brief Called when the server has received a request to append to an - //! attachment's contents. - virtual void AppendAttachment( - const base::FilePath& attachment, const std::string& data) = 0; - //! \brief Called when the server has received a request to remove a file //! from the list of files attached to crash reports. virtual void RemoveAttachment(const base::FilePath& attachment) = 0; diff --git a/util/linux/exception_handler_client.cc b/util/linux/exception_handler_client.cc index 3398fb3eee..306a932e38 100644 --- a/util/linux/exception_handler_client.cc +++ b/util/linux/exception_handler_client.cc @@ -16,7 +16,6 @@ #include #include -#include #include #include #include @@ -27,7 +26,6 @@ #include "build/build_config.h" #include "third_party/lss/lss.h" #include "util/file/file_io.h" -#include "util/file/file_writer.h" #include "util/linux/ptrace_broker.h" #include "util/linux/socket.h" #include "util/misc/from_pointer_cast.h" @@ -64,71 +62,6 @@ class ScopedSigprocmaskRestore { bool mask_is_set_; }; -bool WriteAttachmentFile( - const base::FilePath& attachment, - const std::string& data, - ExceptionHandlerProtocol::AttachmentWriteOperation operation) { - FileWriter writer; - if (!writer.Open(attachment, - operation == ExceptionHandlerProtocol::kAttachmentWriteAppend - ? FileWriteMode::kReuseOrCreate - : FileWriteMode::kTruncateOrCreate, - FilePermissions::kOwnerOnly) || - (operation == ExceptionHandlerProtocol::kAttachmentWriteAppend && - writer.Seek(0, SEEK_END) < 0) || - !writer.Write(data.data(), data.size())) { - LOG(ERROR) << "failed to write attachment " << attachment; - return false; - } - return true; -} - -bool SendAttachmentWrite( - int server_sock, - const base::FilePath& attachment, - const std::string& data, - ExceptionHandlerProtocol::AttachmentWriteOperation operation) { - if (attachment.value().size() + 1 > PATH_MAX) { - LOG(ERROR) << "attachment path too long: " << attachment.value().size() - << " bytes"; - return false; - } - if (data.size() > - ExceptionHandlerProtocol::kMaxAttachmentWritePayloadSize) { - return WriteAttachmentFile(attachment, data, operation); - } - - ExceptionHandlerProtocol::ClientToServerMessage message; - message.type = - ExceptionHandlerProtocol::ClientToServerMessage::kTypeWriteAttachment; - message.attachment_write_info.path_size = - static_cast(attachment.value().size() + 1); - message.attachment_write_info.payload_size = - static_cast(data.size()); - message.attachment_write_info.operation = operation; - - std::string payload(sizeof(message) + attachment.value().size() + 1 + - data.size(), - '\0'); - memcpy(&payload[0], &message, sizeof(message)); - memcpy(&payload[sizeof(message)], attachment.value().c_str(), - attachment.value().size() + 1); - if (!data.empty()) { - memcpy(&payload[sizeof(message) + attachment.value().size() + 1], - data.data(), - data.size()); - } - int result = - UnixCredentialSocket::SendMsg(server_sock, payload.data(), payload.size()); - if (result == 0) { - return true; - } - if (result == EMSGSIZE || result == ENOBUFS) { - return WriteAttachmentFile(attachment, data, operation); - } - return false; -} - } // namespace ExceptionHandlerClient::ExceptionHandlerClient(int sock, bool multiple_clients) @@ -301,24 +234,6 @@ void ExceptionHandlerClient::AddAttachment(const base::FilePath& attachment) { UnixCredentialSocket::SendMsg(server_sock_, &message, sizeof(message)); } -bool ExceptionHandlerClient::WriteAttachment(const base::FilePath& attachment, - const std::string& data) { - return SendAttachmentWrite( - server_sock_, - attachment, - data, - ExceptionHandlerProtocol::kAttachmentWriteReplace); -} - -bool ExceptionHandlerClient::AppendAttachment(const base::FilePath& attachment, - const std::string& data) { - return SendAttachmentWrite( - server_sock_, - attachment, - data, - ExceptionHandlerProtocol::kAttachmentWriteAppend); -} - void ExceptionHandlerClient::RemoveAttachment( const base::FilePath& attachment) { ExceptionHandlerProtocol::ClientToServerMessage message; diff --git a/util/linux/exception_handler_client.h b/util/linux/exception_handler_client.h index 6b74893a4f..a7e4c66dfb 100644 --- a/util/linux/exception_handler_client.h +++ b/util/linux/exception_handler_client.h @@ -18,8 +18,6 @@ #include #include -#include - #include "util/linux/exception_handler_protocol.h" namespace crashpad { @@ -73,14 +71,6 @@ class ExceptionHandlerClient { //! \brief Adds an attachment to the crash report. void AddAttachment(const base::FilePath& attachment); - //! \brief Requests that the handler write an attachment's contents. - bool WriteAttachment( - const base::FilePath& attachment, const std::string& data); - - //! \brief Requests that the handler append to an attachment's contents. - bool AppendAttachment( - const base::FilePath& attachment, const std::string& data); - //! \brief Removes an attachment from the crash report. void RemoveAttachment(const base::FilePath& attachment); diff --git a/util/linux/exception_handler_protocol.h b/util/linux/exception_handler_protocol.h index afbc1f8178..0b6a642f00 100644 --- a/util/linux/exception_handler_protocol.h +++ b/util/linux/exception_handler_protocol.h @@ -64,19 +64,6 @@ class ExceptionHandlerProtocol { char path[PATH_MAX]; }; - static constexpr uint32_t kMaxAttachmentWritePayloadSize = 192 * 1024; - - enum AttachmentWriteOperation : uint32_t { - kAttachmentWriteReplace, - kAttachmentWriteAppend, - }; - - struct AttachmentWriteInformation { - uint32_t path_size; - uint32_t payload_size; - uint32_t operation; - }; - //! \brief The signal used to indicate a crash dump is complete. //! //! When multiple clients share a single socket connection with the handler, @@ -109,9 +96,6 @@ class ExceptionHandlerProtocol { //! \brief Request that the server retry pending report uploads. kTypeRequestRetry, - - //! \brief Request that the server write an attachment's contents. - kTypeWriteAttachment, }; Type type; @@ -125,9 +109,6 @@ class ExceptionHandlerProtocol { //! \brief Valid for type == kAddAttachment || type == kRemoveAttachment AttachmentInformation attachment_info; - - //! \brief Valid for type == kTypeWriteAttachment. - AttachmentWriteInformation attachment_write_info; }; }; diff --git a/util/linux/socket.cc b/util/linux/socket.cc index 6641bb5205..bad5056f9f 100644 --- a/util/linux/socket.cc +++ b/util/linux/socket.cc @@ -101,8 +101,7 @@ bool UnixCredentialSocket::RecvMsg(int fd, void* buf, size_t buf_size, ucred* creds, - std::vector* fds, - ssize_t* bytes_received) { + std::vector* fds) { iovec iov; iov.iov_base = buf; iov.iov_len = buf_size; @@ -121,9 +120,6 @@ bool UnixCredentialSocket::RecvMsg(int fd, PLOG(ERROR) << "recvmsg"; return false; } - if (bytes_received) { - *bytes_received = res; - } ucred* local_creds = nullptr; std::vector local_fds; @@ -182,7 +178,7 @@ bool UnixCredentialSocket::RecvMsg(int fd, return false; } - if (!bytes_received && static_cast(res) != buf_size) { + if (static_cast(res) != buf_size) { LOG(ERROR) << "incorrect payload size " << res; return false; } diff --git a/util/linux/socket.h b/util/linux/socket.h index 3545c0e99a..4bde25c814 100644 --- a/util/linux/socket.h +++ b/util/linux/socket.h @@ -88,8 +88,7 @@ class UnixCredentialSocket { void* buf, size_t buf_size, ucred* creds, - std::vector* fds = nullptr, - ssize_t* bytes_received = nullptr); + std::vector* fds = nullptr); }; } // namespace crashpad diff --git a/util/mach/exception_handler_protocol.h b/util/mach/exception_handler_protocol.h index 7fe9d2df85..cdd3af1d1d 100644 --- a/util/mach/exception_handler_protocol.h +++ b/util/mach/exception_handler_protocol.h @@ -49,10 +49,6 @@ struct ClientToServerMessage { //! \brief Remove a file from the list of files attached to crash reports. //! The payload contains the attachment path. kRemoveAttachment = 3, - - //! \brief Write an attachment's contents. The payload contains a - //! null-terminated attachment path followed by the attachment content. - kWriteAttachment = 4, }; mach_msg_header_t header; From 6ee1048da41c3143b2e0ee36e31ea371332e2b20 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 6 Jul 2026 15:11:38 +0200 Subject: [PATCH 3/8] Clean up Windows --- client/crashpad_client_win.cc | 4 +- util/win/exception_handler_server.cc | 60 +++++++++++++++----- util/win/registration_protocol_win_structs.h | 19 +++---- 3 files changed, 53 insertions(+), 30 deletions(-) diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index f00af7899b..402bd673cb 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -1252,7 +1252,6 @@ bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, static_cast(path_length_bytes); message.attachment_write.payload_length_bytes = static_cast(data.size()); - message.attachment_write.operation = kAttachmentWriteReplace; std::string payload(path_length_bytes + data.size(), '\0'); memcpy(&payload[0], attachment.value().c_str(), path_length_bytes); @@ -1279,12 +1278,11 @@ bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, } ClientToServerMessage message = {}; - message.type = ClientToServerMessage::kWriteAttachment; + message.type = ClientToServerMessage::kAppendAttachment; message.attachment_write.path_length_bytes = static_cast(path_length_bytes); message.attachment_write.payload_length_bytes = static_cast(data.size()); - message.attachment_write.operation = kAttachmentWriteAppend; std::string payload(path_length_bytes + data.size(), '\0'); memcpy(&payload[0], attachment.value().c_str(), path_length_bytes); diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 07b55660bf..a89e9bcb0c 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -528,24 +528,24 @@ static void HandleRemoveAttachmentV2( LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); } -static void HandleWriteAttachment( +static bool ReadAttachment( const internal::PipeServiceContext& service_context, - const ClientToServerMessage& message) { + const ClientToServerMessage& message, + base::FilePath* attachment, + std::string* payload) { const uint32_t path_length_bytes = message.attachment_write.path_length_bytes; const uint32_t payload_length_bytes = message.attachment_write.payload_length_bytes; - const auto operation = - static_cast(message.attachment_write.operation); if (path_length_bytes == 0 || path_length_bytes > kMaxPathBytes || path_length_bytes % sizeof(wchar_t) != 0) { LOG(ERROR) << "Invalid path length: " << path_length_bytes; - return; + return false; } if (payload_length_bytes > UINT32_MAX - path_length_bytes) { LOG(ERROR) << "Invalid attachment write length"; - return; + return false; } const uint32_t request_payload_length_bytes = @@ -556,7 +556,7 @@ static void HandleWriteAttachment( &request_payload[0], request_payload_length_bytes)) { LOG(ERROR) << "Failed to read attachment write"; - return; + return false; } const size_t path_length = path_length_bytes / sizeof(wchar_t) - 1; @@ -567,17 +567,42 @@ static void HandleWriteAttachment( path_length_bytes - sizeof(wchar_t)); } - std::string payload( - request_payload.data() + path_length_bytes, payload_length_bytes); + *attachment = base::FilePath(path); + *payload = + std::string(request_payload.data() + path_length_bytes, + payload_length_bytes); + return true; +} + +static void HandleWriteAttachment( + const internal::PipeServiceContext& service_context, + const ClientToServerMessage& message) { + base::FilePath attachment; + std::string payload; + if (!ReadAttachment( + service_context, message, &attachment, &payload)) { + return; + } ServerToClientMessage response = {}; - if (operation == kAttachmentWriteAppend) { - service_context.delegate()->ExceptionHandlerServerAttachmentAppended( - base::FilePath(path), payload); - } else { - service_context.delegate()->ExceptionHandlerServerAttachmentWritten( - base::FilePath(path), payload); + service_context.delegate()->ExceptionHandlerServerAttachmentWritten( + attachment, payload); + LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); +} + +static void HandleAppendAttachment( + const internal::PipeServiceContext& service_context, + const ClientToServerMessage& message) { + base::FilePath attachment; + std::string payload; + if (!ReadAttachment( + service_context, message, &attachment, &payload)) { + return; } + + ServerToClientMessage response = {}; + service_context.delegate()->ExceptionHandlerServerAttachmentAppended( + attachment, payload); LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); } @@ -669,6 +694,11 @@ bool ExceptionHandlerServer::ServiceClientConnection( return false; } + case ClientToServerMessage::kAppendAttachment: { + HandleAppendAttachment(service_context, message); + return false; + } + case ClientToServerMessage::kRequestRetry: { if (!RuntimeMessageOriginIsOwner(service_context)) { return false; diff --git a/util/win/registration_protocol_win_structs.h b/util/win/registration_protocol_win_structs.h index e565bbdea5..96a378de9b 100644 --- a/util/win/registration_protocol_win_structs.h +++ b/util/win/registration_protocol_win_structs.h @@ -132,26 +132,18 @@ struct AttachmentRequestV2 { uint32_t path_length_bytes; }; -enum AttachmentWriteOperation : uint32_t { - kAttachmentWriteReplace, - kAttachmentWriteAppend, -}; - //! \brief A variable-length attachment write request header. //! -//! For kWriteAttachment, the message consists of a ClientToServerMessage -//! with this header in the union, followed by path_length_bytes of wchar_t data -//! containing the null-terminated path and payload_length_bytes of attachment -//! content. +//! For kWriteAttachment and kAppendAttachment, the message consists of a +//! ClientToServerMessage with this header in the union, followed by +//! path_length_bytes of wchar_t data containing the null-terminated path and +//! payload_length_bytes of attachment content. struct AttachmentWriteRequest { //! \brief Length of the path in bytes, including null terminator. uint32_t path_length_bytes; //! \brief Length of the attachment content in bytes. uint32_t payload_length_bytes; - - //! \brief The write operation to apply to the attachment content. - uint32_t operation; }; //! follow the maximum path length documented here: @@ -192,6 +184,9 @@ struct ClientToServerMessage { //! \brief For AttachmentWriteRequest. kWriteAttachment, + + //! \brief For AttachmentWriteRequest. + kAppendAttachment, } type; union { From 2ea1030687ff89cc3c4bee9831b53b3f304e2ad1 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 8 Jul 2026 10:03:35 +0200 Subject: [PATCH 4/8] don't wait for write - defeats the purpose --- util/win/exception_handler_server.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index a89e9bcb0c..47712dbb75 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -585,9 +585,11 @@ static void HandleWriteAttachment( } ServerToClientMessage response = {}; + if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) { + return; + } service_context.delegate()->ExceptionHandlerServerAttachmentWritten( attachment, payload); - LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); } static void HandleAppendAttachment( @@ -601,9 +603,11 @@ static void HandleAppendAttachment( } ServerToClientMessage response = {}; + if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) { + return; + } service_context.delegate()->ExceptionHandlerServerAttachmentAppended( attachment, payload); - LoggingWriteFile(service_context.pipe(), &response, sizeof(response)); } // This function must be called with service_context.pipe() already connected to From 3f940c03586c8cf34f5e42d6ac3850687665b024 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Wed, 8 Jul 2026 10:18:32 +0200 Subject: [PATCH 5/8] cap payload size --- client/crashpad_client_win.cc | 2 ++ util/win/exception_handler_server.cc | 5 +++++ util/win/registration_protocol_win_structs.h | 3 +++ 3 files changed, 10 insertions(+) diff --git a/client/crashpad_client_win.cc b/client/crashpad_client_win.cc index 402bd673cb..00e8697144 100644 --- a/client/crashpad_client_win.cc +++ b/client/crashpad_client_win.cc @@ -1241,6 +1241,7 @@ bool CrashpadClient::WriteAttachment(const base::FilePath& attachment, const size_t path_length_bytes = (attachment.value().length() + 1) * sizeof(wchar_t); if (path_length_bytes > kMaxPathBytes || + data.size() > kMaxAttachmentPayloadBytes || data.size() > UINT32_MAX - path_length_bytes) { LOG(ERROR) << "attachment content too large"; return false; @@ -1272,6 +1273,7 @@ bool CrashpadClient::AppendAttachment(const base::FilePath& attachment, const size_t path_length_bytes = (attachment.value().length() + 1) * sizeof(wchar_t); if (path_length_bytes > kMaxPathBytes || + data.size() > kMaxAttachmentPayloadBytes || data.size() > UINT32_MAX - path_length_bytes) { LOG(ERROR) << "attachment content too large"; return false; diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 47712dbb75..99df188fae 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -543,6 +543,11 @@ static bool ReadAttachment( LOG(ERROR) << "Invalid path length: " << path_length_bytes; return false; } + if (payload_length_bytes > kMaxAttachmentPayloadBytes) { + LOG(ERROR) << "Invalid attachment payload length: " + << payload_length_bytes; + return false; + } if (payload_length_bytes > UINT32_MAX - path_length_bytes) { LOG(ERROR) << "Invalid attachment write length"; return false; diff --git a/util/win/registration_protocol_win_structs.h b/util/win/registration_protocol_win_structs.h index 96a378de9b..2c7b3246c4 100644 --- a/util/win/registration_protocol_win_structs.h +++ b/util/win/registration_protocol_win_structs.h @@ -150,6 +150,9 @@ struct AttachmentWriteRequest { //! https://learn.microsoft.com/en-us/windows/win32/fileio/maximum-file-path-limitation constexpr uint32_t kMaxPathBytes = 32768 * sizeof(wchar_t); +//! \brief Maximum variable-length attachment content payload, in bytes. +constexpr uint32_t kMaxAttachmentPayloadBytes = 100 * 1024 * 1024; + //! \brief The message passed from client to server by //! SendToCrashHandlerServer(). struct ClientToServerMessage { From e3f29f1571a063f385b8ae0431ac45b860e0a79f Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 10 Jul 2026 08:46:52 +0200 Subject: [PATCH 6/8] add owner checks to the new IPC methods --- util/win/exception_handler_server.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index 99df188fae..dd2d9a2ec9 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -699,11 +699,17 @@ bool ExceptionHandlerServer::ServiceClientConnection( } case ClientToServerMessage::kWriteAttachment: { + if (!RuntimeMessageOriginIsOwner(service_context)) { + return false; + } HandleWriteAttachment(service_context, message); return false; } case ClientToServerMessage::kAppendAttachment: { + if (!RuntimeMessageOriginIsOwner(service_context)) { + return false; + } HandleAppendAttachment(service_context, message); return false; } From 88b021cf0c40a0b9a8432f35b3ad78aeb1900e0b Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 10 Jul 2026 09:01:36 +0200 Subject: [PATCH 7/8] fix typo --- client/crashpad_client_mac.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/crashpad_client_mac.cc b/client/crashpad_client_mac.cc index e1e7469cc2..25d5b04db4 100644 --- a/client/crashpad_client_mac.cc +++ b/client/crashpad_client_mac.cc @@ -20,7 +20,7 @@ #include #include #include -#include +#include #include #include From 16981c915344d056c25abf1358f3c43777db313c Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 10 Jul 2026 13:27:33 +0200 Subject: [PATCH 8/8] add clarifying comments --- util/win/exception_handler_server.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/util/win/exception_handler_server.cc b/util/win/exception_handler_server.cc index dd2d9a2ec9..a185063ca4 100644 --- a/util/win/exception_handler_server.cc +++ b/util/win/exception_handler_server.cc @@ -589,6 +589,9 @@ static void HandleWriteAttachment( return; } + // Acknowledge IPC payload acceptance before disk I/O. This message exists to + // offload potentially slow disk I/O from the client; waiting for the file + // write would make the client block on the work this API is meant to avoid. ServerToClientMessage response = {}; if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) { return; @@ -607,6 +610,9 @@ static void HandleAppendAttachment( return; } + // Acknowledge IPC payload acceptance before disk I/O. This message exists to + // offload potentially slow disk I/O from the client; waiting for the file + // append would make the client block on the work this API is meant to avoid. ServerToClientMessage response = {}; if (!LoggingWriteFile(service_context.pipe(), &response, sizeof(response))) { return;