tor-browser

The Tor Browser
git clone https://git.dasho.dev/tor-browser.git
Log | Files | Refs | README | LICENSE

commit 764284c7fc09766d7448b10d8fc09a77daa29181
parent af8df68927a9db0a0eb0392356355e182bc78ab9
Author: Gabriele Svelto <gsvelto@mozilla.com>
Date:   Thu, 27 Nov 2025 15:29:52 +0000

Bug 1989686 - Refactor errors in per-type groups r=afranchuk

Differential Revision: https://phabricator.services.mozilla.com/D270529

Diffstat:
Mtoolkit/crashreporter/crash_helper_common/src/breakpad/unix_strings.rs | 2+-
Mtoolkit/crashreporter/crash_helper_common/src/breakpad/windows_strings.rs | 2+-
Mtoolkit/crashreporter/crash_helper_common/src/errors.rs | 66+++++++++++++++++++++---------------------------------------------
Mtoolkit/crashreporter/crash_helper_common/src/ipc_channel.rs | 18++++++++++++++++++
Mtoolkit/crashreporter/crash_helper_common/src/ipc_channel/unix.rs | 10+++++-----
Mtoolkit/crashreporter/crash_helper_common/src/ipc_channel/windows.rs | 27++++++++++++++++-----------
Mtoolkit/crashreporter/crash_helper_common/src/ipc_connector/unix.rs | 50+++++++++++++++++++++++---------------------------
Mtoolkit/crashreporter/crash_helper_common/src/ipc_connector/windows.rs | 69+++++++++++++++++++++++++++++++++++++++++++--------------------------
Mtoolkit/crashreporter/crash_helper_common/src/ipc_listener.rs | 24++++++++++++++++++++++++
Mtoolkit/crashreporter/crash_helper_common/src/ipc_listener/windows.rs | 50++++++++++++++++++++++++++++++++------------------
Mtoolkit/crashreporter/crash_helper_common/src/lib.rs | 6+++---
Mtoolkit/crashreporter/crash_helper_common/src/messages.rs | 28+++++++++++++++++++++++-----
Mtoolkit/crashreporter/crash_helper_common/src/platform.rs | 6+++---
Mtoolkit/crashreporter/crash_helper_common/src/platform/linux.rs | 40++++++++++++++++++++++++++++++++++------
Mtoolkit/crashreporter/crash_helper_common/src/platform/macos.rs | 37+++++++++++++++++++++++++++++++------
Mtoolkit/crashreporter/crash_helper_common/src/platform/windows.rs | 109+++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------------------
Mtoolkit/crashreporter/crash_helper_server/src/ipc_server/unix.rs | 4++--
Mtoolkit/crashreporter/crash_helper_server/src/ipc_server/windows.rs | 6++++--
18 files changed, 363 insertions(+), 191 deletions(-)

diff --git a/toolkit/crashreporter/crash_helper_common/src/breakpad/unix_strings.rs b/toolkit/crashreporter/crash_helper_common/src/breakpad/unix_strings.rs @@ -9,7 +9,7 @@ use std::{ os::unix::ffi::OsStringExt, }; -use crate::{errors::MessageError, BreakpadString}; +use crate::{messages::MessageError, BreakpadString}; use super::BreakpadChar; diff --git a/toolkit/crashreporter/crash_helper_common/src/breakpad/windows_strings.rs b/toolkit/crashreporter/crash_helper_common/src/breakpad/windows_strings.rs @@ -9,7 +9,7 @@ use std::{ os::windows::ffi::{OsStrExt, OsStringExt}, }; -use crate::{errors::MessageError, BreakpadChar, BreakpadString}; +use crate::{messages::MessageError, BreakpadChar, BreakpadString}; // BreakpadString trait implementation for Windows native UTF-16 strings impl BreakpadString for OsString { diff --git a/toolkit/crashreporter/crash_helper_common/src/errors.rs b/toolkit/crashreporter/crash_helper_common/src/errors.rs @@ -2,63 +2,39 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -use std::{ - array::TryFromSliceError, - ffi::{FromBytesWithNulError, NulError}, - num::TryFromIntError, -}; +use std::num::TryFromIntError; use thiserror::Error; #[cfg(not(target_os = "windows"))] -use nix::errno::Errno as SystemError; +pub use nix::errno::Errno as SystemError; #[cfg(target_os = "windows")] -use windows_sys::Win32::Foundation::WIN32_ERROR as SystemError; +pub use windows_sys::Win32::Foundation::WIN32_ERROR as SystemError; + +use crate::{ + messages::{self, MessageError}, + platform::PlatformError, +}; #[derive(Debug, Error)] pub enum IPCError { #[error("Message error")] BadMessage(#[from] MessageError), - #[error("Generic system error: {0}")] - System(SystemError), - #[error("Could not bind socket to an address, error: {0}")] - BindFailed(SystemError), - #[error("Could not listen on a socket, error: {0}")] - ListenFailed(SystemError), - #[error("Could not accept an incoming connection, error: {0}")] - AcceptFailed(SystemError), - #[error("Could not connect to a socket, error: {0}")] + #[error("Could not connect to a socket: {0}")] ConnectionFailure(SystemError), - #[error("Could not send data, error: {0}")] - TransmissionFailure(SystemError), - #[error("Could not receive data, error: {0}")] - ReceptionFailure(SystemError), - #[error("Error while waiting for events, error: {0:?}")] - WaitingFailure(Option<SystemError>), + #[error("Failed to create a connector: {0}")] + CreationFailure(PlatformError), #[error("Buffer length exceeds a 32-bit integer")] InvalidSize(#[from] TryFromIntError), #[error("Error while parsing a file descriptor string")] ParseError, - #[error("Failed to duplicate clone handle")] - CloneHandleFailed(#[source] std::io::Error), - #[cfg(target_os = "windows")] - #[error("Missing destination process handle")] - MissingProcessHandle, -} - -#[derive(Debug, Error)] -pub enum MessageError { - #[error("Truncated message")] - Truncated, - #[error("Message kind is invalid")] - InvalidKind, - #[error("The message contained an invalid payload")] - InvalidData, - #[error("Missing ancillary data")] - MissingAncillary, - #[error("Invalid message size")] - InvalidSize(#[from] TryFromSliceError), - #[error("Missing nul terminator")] - MissingNul(#[from] FromBytesWithNulError), - #[error("Missing nul terminator")] - InteriorNul(#[from] NulError), + #[error("Could not receive data: {0}")] + ReceptionFailure(PlatformError), + #[error("An operation timed out")] + Timeout, + #[error("Could not send data: {0}")] + TransmissionFailure(PlatformError), + #[error("Unexpected message of kind: {0:?}")] + UnexpectedMessage(messages::Kind), + #[error("Error while waiting for events, error: {0:?}")] + WaitingFailure(Option<SystemError>), } diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_channel.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_channel.rs @@ -2,6 +2,24 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use thiserror::Error; + +use crate::{errors::IPCError, IPCListenerError, PlatformError}; + +/***************************************************************************** + * Error definitions * + *****************************************************************************/ + +#[derive(Debug, Error)] +pub enum IPCChannelError { + #[error("Could not create connector: {0}")] + Connector(#[from] IPCError), + #[error("Could not create a listener: {0}")] + Listener(#[from] IPCListenerError), + #[error("Could not create a socketpair: {0}")] + SocketPair(#[from] PlatformError), +} + /***************************************************************************** * Windows * *****************************************************************************/ diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_channel/unix.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_channel/unix.rs @@ -8,7 +8,7 @@ use std::process; use crate::platform::linux::unix_socketpair; #[cfg(target_os = "macos")] use crate::platform::macos::unix_socketpair; -use crate::{errors::IPCError, IPCConnector, IPCListener, Pid}; +use crate::{ipc_channel::IPCChannelError, IPCConnector, IPCListener, Pid}; pub struct IPCChannel { listener: IPCListener, @@ -21,11 +21,11 @@ impl IPCChannel { /// will use the current process PID as part of its address and two /// connected endpoints. The listener and the server-side endpoint can be /// inherited by a child process, the client-side endpoint cannot. - pub fn new() -> Result<IPCChannel, IPCError> { + pub fn new() -> Result<IPCChannel, IPCChannelError> { let listener = IPCListener::new(process::id() as Pid)?; // Only the server-side socket will be left open after an exec(). - let pair = unix_socketpair().map_err(IPCError::System)?; + let pair = unix_socketpair().map_err(IPCChannelError::SocketPair)?; let client_endpoint = IPCConnector::from_fd(pair.0)?; let server_endpoint = IPCConnector::from_fd_inheritable(pair.1)?; @@ -52,8 +52,8 @@ pub struct IPCClientChannel { impl IPCClientChannel { /// Create a new IPC channel for use between one of the browser's child /// processes and the crash helper. - pub fn new() -> Result<IPCClientChannel, IPCError> { - let pair = unix_socketpair().map_err(IPCError::System)?; + pub fn new() -> Result<IPCClientChannel, IPCChannelError> { + let pair = unix_socketpair().map_err(IPCChannelError::SocketPair)?; let client_endpoint = IPCConnector::from_fd(pair.0)?; let server_endpoint = IPCConnector::from_fd(pair.1)?; diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_channel/windows.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_channel/windows.rs @@ -4,12 +4,12 @@ use std::{ffi::CString, hash::RandomState, process}; -use windows_sys::Win32::Foundation::ERROR_ACCESS_DENIED; +use windows_sys::Win32::Foundation::{ERROR_ACCESS_DENIED, ERROR_ADDRESS_ALREADY_ASSOCIATED}; use crate::{ - errors::IPCError, - platform::windows::{get_last_error, server_addr}, - IPCConnector, IPCListener, Pid, + ipc_channel::IPCChannelError, + platform::{windows::server_addr, PlatformError}, + IPCConnector, IPCListener, IPCListenerError, Pid, }; pub struct IPCChannel { @@ -22,7 +22,7 @@ impl IPCChannel { /// Create a new IPCChannel, this includes a listening endpoint that /// will use the current process PID as part of its address and two /// connected endpoints. - pub fn new() -> Result<IPCChannel, IPCError> { + pub fn new() -> Result<IPCChannel, IPCChannelError> { let pid = process::id() as Pid; let mut listener = IPCListener::new(server_addr(pid))?; listener.listen()?; @@ -52,7 +52,7 @@ pub struct IPCClientChannel { impl IPCClientChannel { /// Create a new IPC channel for use between one of the browser's child /// processes and the crash helper. - pub fn new() -> Result<IPCClientChannel, IPCError> { + pub fn new() -> Result<IPCClientChannel, IPCChannelError> { let mut listener = Self::create_listener()?; listener.listen()?; let client_endpoint = IPCConnector::connect(listener.address())?; @@ -64,7 +64,7 @@ impl IPCClientChannel { }) } - fn create_listener() -> Result<IPCListener, IPCError> { + fn create_listener() -> Result<IPCListener, IPCListenerError> { const ATTEMPTS: u32 = 5; // We pick the listener name at random, as unlikely as it may be there @@ -79,14 +79,19 @@ impl IPCClientChannel { .unwrap(); match IPCListener::new(pipe_name) { Ok(listener) => return Ok(listener), - Err(_error @ IPCError::System(ERROR_ACCESS_DENIED)) => {} // Try again + Err( + _error @ IPCListenerError::CreationError(PlatformError::CreatePipeFailure( + ERROR_ACCESS_DENIED, + )), + ) => {} // Try again Err(error) => return Err(error), } } - // If we got to this point return whatever was the last error we - // encountered along the way. - Err(IPCError::System(get_last_error())) + // If we got to this point give up. + Err(IPCListenerError::CreationError( + PlatformError::CreatePipeFailure(ERROR_ADDRESS_ALREADY_ASSOCIATED), + )) } /// Deconstruct the IPC channel, returning the listening endpoint, diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_connector/unix.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_connector/unix.rs @@ -6,7 +6,7 @@ use crate::platform::linux::{set_socket_cloexec, set_socket_default_flags}; #[cfg(target_os = "macos")] use crate::platform::macos::{set_socket_cloexec, set_socket_default_flags}; -use crate::{ignore_eintr, IntoRawAncillaryData, ProcessHandle, IO_TIMEOUT}; +use crate::{ignore_eintr, IntoRawAncillaryData, PlatformError, ProcessHandle, IO_TIMEOUT}; use nix::{ cmsg_space, @@ -48,7 +48,7 @@ impl IPCConnector { /// will not be possible to inerhit this connector in a child process. pub(crate) fn from_fd(socket: OwnedFd) -> Result<IPCConnector, IPCError> { let connector = IPCConnector::from_fd_inheritable(socket)?; - set_socket_cloexec(connector.socket.as_fd()).map_err(IPCError::System)?; + set_socket_cloexec(connector.socket.as_fd()).map_err(IPCError::CreationFailure)?; Ok(connector) } @@ -56,7 +56,7 @@ impl IPCConnector { /// `FD_CLOEXEC` flag will not be set on the underlying socket and thus it /// will be possible to inherit this connector in a child process. pub(crate) fn from_fd_inheritable(socket: OwnedFd) -> Result<IPCConnector, IPCError> { - set_socket_default_flags(socket.as_fd()).map_err(IPCError::System)?; + set_socket_default_flags(socket.as_fd()).map_err(IPCError::CreationFailure)?; Ok(IPCConnector { socket }) } @@ -106,15 +106,15 @@ impl IPCConnector { self.socket.as_fd() } - pub fn poll(&self, flags: PollFlags) -> Result<(), Errno> { + pub fn poll(&self, flags: PollFlags) -> Result<(), PlatformError> { let timeout = PollTimeout::from(IO_TIMEOUT); let res = ignore_eintr!(poll( &mut [PollFd::new(self.socket.as_fd(), flags)], timeout )); match res { - Err(e) => Err(e), - Ok(_res @ 0) => Err(Errno::EAGAIN), + Err(e) => Err(PlatformError::PollFailure(e)), + Ok(_res @ 0) => Err(PlatformError::PollFailure(Errno::EAGAIN)), Ok(_) => Ok(()), } } @@ -137,14 +137,14 @@ impl IPCConnector { let header = self.recv_header()?; if header.kind != T::kind() { - return Err(IPCError::ReceptionFailure(Errno::EBADMSG)); + return Err(IPCError::UnexpectedMessage(header.kind)); } - let (data, _) = self.recv(header.size).map_err(IPCError::ReceptionFailure)?; + let (data, _) = self.recv(header.size)?; T::decode(&data, None).map_err(IPCError::from) } - fn send_nonblock(&self, buff: &[u8], fd: &Option<AncillaryData>) -> Result<(), Errno> { + fn send_nonblock(&self, buff: &[u8], fd: &Option<AncillaryData>) -> Result<(), PlatformError> { let iov = [IoSlice::new(buff)]; let scm_fds: Vec<i32> = fd.iter().map(|fd| fd.as_raw_fd()).collect(); let scm = ControlMessage::ScmRights(&scm_fds); @@ -162,19 +162,17 @@ impl IPCConnector { if bytes_sent == buff.len() { Ok(()) } else { - // TODO: This should never happen but we might want to put a - // better error message here. - Err(Errno::EMSGSIZE) + Err(PlatformError::SendTooShort(buff.len(), bytes_sent)) } } - Err(code) => Err(code), + Err(code) => Err(PlatformError::SendFailure(code)), } } - fn send(&self, buff: &[u8], fd: Option<AncillaryData>) -> Result<(), Errno> { + fn send(&self, buff: &[u8], fd: Option<AncillaryData>) -> Result<(), PlatformError> { let res = self.send_nonblock(buff, &fd); match res { - Err(_code @ Errno::EAGAIN) => { + Err(PlatformError::SendFailure(Errno::EAGAIN)) => { // If the socket was not ready to send data wait for it to // become unblocked then retry sending just once. self.poll(PollFlags::POLLOUT)?; @@ -185,16 +183,14 @@ impl IPCConnector { } pub fn recv_header(&self) -> Result<messages::Header, IPCError> { - let (header, _) = self - .recv(messages::HEADER_SIZE) - .map_err(IPCError::ReceptionFailure)?; + let (header, _) = self.recv(messages::HEADER_SIZE)?; messages::Header::decode(&header).map_err(IPCError::BadMessage) } fn recv_nonblock( &self, expected_size: usize, - ) -> Result<(Vec<u8>, Option<AncillaryData>), Errno> { + ) -> Result<(Vec<u8>, Option<AncillaryData>), PlatformError> { let mut buff: Vec<u8> = vec![0; expected_size]; let mut cmsg_buffer = cmsg_space!(RawFd); let mut iov = [IoSliceMut::new(&mut buff)]; @@ -221,7 +217,7 @@ impl IPCConnector { Some(&mut cmsg_buffer), MsgFlags::empty(), ))?, - Err(e) => return Err(e), + Err(e) => return Err(PlatformError::ReceiveFailure(e)), Ok(val) => val, }; @@ -229,31 +225,31 @@ impl IPCConnector { if let ControlMessageOwned::ScmRights(fds) = cmsg { fds.first().map(|&fd| unsafe { OwnedFd::from_raw_fd(fd) }) } else { - return Err(Errno::EBADMSG); + return Err(PlatformError::ReceiveMissingCredentials); } } else { None }; if res.bytes != expected_size { - // TODO: This should only ever happen if the other side has gone rogue, - // we need a better error message here. - return Err(Errno::EBADMSG); + return Err(PlatformError::ReceiveTooShort(expected_size, res.bytes)); } Ok((buff, fd)) } - pub fn recv(&self, expected_size: usize) -> Result<(Vec<u8>, Option<AncillaryData>), Errno> { + pub fn recv(&self, expected_size: usize) -> Result<(Vec<u8>, Option<AncillaryData>), IPCError> { let res = self.recv_nonblock(expected_size); match res { - Err(_code @ Errno::EAGAIN) => { + Err(PlatformError::ReceiveFailure(Errno::EAGAIN)) => { // If the socket was not ready to receive data wait for it to // become unblocked then retry receiving just once. - self.poll(PollFlags::POLLIN)?; + self.poll(PollFlags::POLLIN) + .map_err(IPCError::ReceptionFailure)?; self.recv_nonblock(expected_size) } _ => res, } + .map_err(IPCError::ReceptionFailure) } } diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_connector/windows.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_connector/windows.rs @@ -5,7 +5,10 @@ use crate::{ errors::IPCError, messages::{self, Message}, - platform::windows::{create_manual_reset_event, get_last_error, OverlappedOperation}, + platform::{ + windows::{create_manual_reset_event, get_last_error, OverlappedOperation}, + PlatformError, + }, IntoRawAncillaryData, IO_TIMEOUT, }; @@ -20,9 +23,8 @@ use std::{ }; use windows_sys::Win32::{ Foundation::{ - DuplicateHandle, GetLastError, DUPLICATE_CLOSE_SOURCE, DUPLICATE_SAME_ACCESS, - ERROR_FILE_NOT_FOUND, ERROR_INVALID_MESSAGE, ERROR_PIPE_BUSY, FALSE, HANDLE, - INVALID_HANDLE_VALUE, WAIT_TIMEOUT, + DuplicateHandle, DUPLICATE_CLOSE_SOURCE, DUPLICATE_SAME_ACCESS, ERROR_FILE_NOT_FOUND, + ERROR_INVALID_MESSAGE, ERROR_PIPE_BUSY, FALSE, HANDLE, INVALID_HANDLE_VALUE, }, Security::SECURITY_ATTRIBUTES, Storage::FileSystem::{ @@ -81,7 +83,7 @@ pub struct IPCConnector { impl IPCConnector { pub fn from_ancillary(handle: OwnedHandle) -> Result<IPCConnector, IPCError> { - let event = create_manual_reset_event()?; + let event = create_manual_reset_event().map_err(IPCError::CreationFailure)?; Ok(IPCConnector { handle: Rc::new(handle), @@ -144,10 +146,10 @@ impl IPCConnector { let elapsed = now.elapsed(); if elapsed >= timeout { - return Err(IPCError::System(WAIT_TIMEOUT)); // TODO: We need a dedicated error + return Err(IPCError::Timeout); } - let error = unsafe { GetLastError() }; + let error = get_last_error(); // The pipe might have not been created yet or it might be busy. if (error == ERROR_FILE_NOT_FOUND) || (error == ERROR_PIPE_BUSY) { @@ -158,14 +160,14 @@ impl IPCConnector { (timeout - elapsed).as_millis() as u32, ) }; - let error = unsafe { GetLastError() }; + let error = get_last_error(); // If the pipe hasn't been created yet loop over and try again if (res == FALSE) && (error != ERROR_FILE_NOT_FOUND) { - return Err(IPCError::System(error)); + return Err(IPCError::ConnectionFailure(error)); } } else { - return Err(IPCError::System(error)); + return Err(IPCError::ConnectionFailure(error)); } } @@ -182,7 +184,7 @@ impl IPCConnector { ) }; if res == FALSE { - return Err(IPCError::System(unsafe { GetLastError() })); + return Err(IPCError::ConnectionFailure(get_last_error())); } // SAFETY: We've verified above that the pipe handle is valid @@ -234,7 +236,9 @@ impl IPCConnector { let header = self.recv_header()?; if header.kind != T::kind() { - return Err(IPCError::ReceptionFailure(ERROR_INVALID_MESSAGE)); + return Err(IPCError::ReceptionFailure( + crate::platform::PlatformError::IOError(ERROR_INVALID_MESSAGE), + )); } let (data, _) = self.recv(header.size)?; @@ -252,11 +256,14 @@ impl IPCConnector { return Ok(()); } - self.overlapped = Some(OverlappedOperation::sched_recv( - &self.handle, - self.event_raw_handle(), - HANDLE_SIZE + messages::HEADER_SIZE, - )?); + self.overlapped = Some( + OverlappedOperation::sched_recv( + &self.handle, + self.event_raw_handle(), + HANDLE_SIZE + messages::HEADER_SIZE, + ) + .map_err(IPCError::ReceptionFailure)?, + ); Ok(()) } @@ -264,14 +271,17 @@ impl IPCConnector { // We should never call collect_header() on a connector that wasn't // waiting for one, so panic in that scenario. let overlapped = self.overlapped.take().unwrap(); - let buffer = overlapped.collect_recv(/* wait */ false)?; + let buffer = overlapped + .collect_recv(/* wait */ false) + .map_err(IPCError::ReceptionFailure)?; let (data, _) = extract_buffer_and_handle(buffer)?; messages::Header::decode(data.as_ref()).map_err(IPCError::BadMessage) } fn send(&self, buff: &[u8], handle: Option<AncillaryData>) -> Result<(), IPCError> { let handle = if let Some(handle) = handle { - self.clone_handle(handle)? + self.clone_handle(handle) + .map_err(IPCError::TransmissionFailure)? } else { INVALID_ANCILLARY_DATA }; @@ -281,8 +291,12 @@ impl IPCConnector { buffer.extend(buff); let overlapped = - OverlappedOperation::sched_send(&self.handle, self.event_raw_handle(), buffer)?; - overlapped.complete_send(/* wait */ true) + OverlappedOperation::sched_send(&self.handle, self.event_raw_handle(), buffer) + .map_err(IPCError::TransmissionFailure)?; + + overlapped + .complete_send(/* wait */ true) + .map_err(IPCError::TransmissionFailure) } pub fn recv(&self, expected_size: usize) -> Result<(Vec<u8>, Option<AncillaryData>), IPCError> { @@ -290,8 +304,11 @@ impl IPCConnector { &self.handle, self.event_raw_handle(), HANDLE_SIZE + expected_size, - )?; - let buffer = overlapped.collect_recv(/* wait */ true)?; + ) + .map_err(IPCError::ReceptionFailure)?; + let buffer = overlapped + .collect_recv(/* wait */ true) + .map_err(IPCError::ReceptionFailure)?; extract_buffer_and_handle(buffer) } @@ -299,9 +316,9 @@ impl IPCConnector { /// transfer handles over this connector. Note that this consumes the /// incoming handle because we want it to be closed after it's been cloned /// over to the other process. - fn clone_handle(&self, handle: OwnedHandle) -> Result<HANDLE, IPCError> { + fn clone_handle(&self, handle: OwnedHandle) -> Result<HANDLE, PlatformError> { let Some(dst_process) = self.process.as_ref() else { - return Err(IPCError::MissingProcessHandle); + return Err(PlatformError::MissingProcessHandle); }; let mut dst_handle: HANDLE = INVALID_ANCILLARY_DATA; let res = unsafe { @@ -317,7 +334,7 @@ impl IPCConnector { }; if res == 0 { - return Err(IPCError::CloneHandleFailed(Error::from_raw_os_error( + return Err(PlatformError::CloneHandleFailed(Error::from_raw_os_error( get_last_error() as i32, ))); } diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_listener.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_listener.rs @@ -2,6 +2,30 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use thiserror::Error; + +use crate::{errors::IPCError, platform::PlatformError}; + +/***************************************************************************** + * Error definitions * + *****************************************************************************/ + +#[derive(Debug, Error)] +pub enum IPCListenerError { + #[error("Could not accept incoming connection: {0}")] + AcceptError(PlatformError), + #[error("Issue with an underlying connector: {0}")] + ConnectorError(#[from] IPCError), + #[error("Could not create listener: {0}")] + CreationError(PlatformError), + #[error("Could not listen for incoming connections: {0}")] + ListenError(PlatformError), + #[error("Could not parse handle: {0}")] + ParseError(String), + #[error("Could not create a new pipe: {0}")] + PipeCreationFailure(PlatformError), +} + /***************************************************************************** * Windows * *****************************************************************************/ diff --git a/toolkit/crashreporter/crash_helper_common/src/ipc_listener/windows.rs b/toolkit/crashreporter/crash_helper_common/src/ipc_listener/windows.rs @@ -4,8 +4,11 @@ use crate::{ errors::IPCError, - platform::windows::{create_manual_reset_event, server_addr, OverlappedOperation}, - IPCConnector, Pid, + platform::{ + windows::{create_manual_reset_event, server_addr, OverlappedOperation}, + PlatformError, + }, + IPCConnector, IPCListenerError, Pid, }; use std::{ @@ -39,9 +42,10 @@ pub struct IPCListener { } impl IPCListener { - pub(crate) fn new(server_addr: CString) -> Result<IPCListener, IPCError> { - let pipe = create_named_pipe(&server_addr, /* first_instance */ true)?; - let event = create_manual_reset_event()?; + pub(crate) fn new(server_addr: CString) -> Result<IPCListener, IPCListenerError> { + let pipe = create_named_pipe(&server_addr, /* first_instance */ true) + .map_err(IPCListenerError::PipeCreationFailure)?; + let event = create_manual_reset_event().map_err(IPCListenerError::AcceptError)?; Ok(IPCListener { server_addr, @@ -59,15 +63,15 @@ impl IPCListener { &self.server_addr } - pub(crate) fn listen(&mut self) -> Result<(), IPCError> { - self.overlapped = Some(OverlappedOperation::listen( - &self.handle, - self.event_raw_handle(), - )?); + pub(crate) fn listen(&mut self) -> Result<(), IPCListenerError> { + self.overlapped = Some( + OverlappedOperation::listen(&self.handle, self.event_raw_handle()) + .map_err(IPCListenerError::ListenError)?, + ); Ok(()) } - pub fn accept(&mut self) -> Result<IPCConnector, IPCError> { + pub fn accept(&mut self) -> Result<IPCConnector, IPCListenerError> { let connected_pipe = { // We extract the overlapped operation within this scope so // that it's dropped right away. This ensures that only one @@ -76,8 +80,13 @@ impl IPCListener { .overlapped .take() .expect("Accepting a connection without listening first"); - overlapped.accept(self.handle.as_raw_handle() as HANDLE)?; - let new_pipe = create_named_pipe(&self.server_addr, /* first_instance */ false)?; + overlapped + .accept(self.handle.as_raw_handle() as HANDLE) + .map_err(IPCListenerError::AcceptError)?; + + let new_pipe = create_named_pipe(&self.server_addr, /* first_instance */ false) + .map_err(IPCListenerError::PipeCreationFailure)?; + std::mem::replace(&mut self.handle, Rc::new(new_pipe)) }; @@ -88,7 +97,9 @@ impl IPCListener { // We can guarantee that there's only one reference to this handle at // this point in time. - IPCConnector::from_ancillary(Rc::<OwnedHandle>::try_unwrap(connected_pipe).unwrap()) + Ok(IPCConnector::from_ancillary( + Rc::<OwnedHandle>::try_unwrap(connected_pipe).unwrap(), + )?) } /// Serialize this listener into a string that can be passed on the @@ -101,13 +112,13 @@ impl IPCListener { /// Deserialize a listener from an argument passed on the command-line. /// The resulting listener is ready to accept new connections. - pub fn deserialize(string: &CStr, pid: Pid) -> Result<IPCListener, IPCError> { + pub fn deserialize(string: &CStr, pid: Pid) -> Result<IPCListener, IPCListenerError> { let server_addr = server_addr(pid); let string = string.to_str().map_err(|_e| IPCError::ParseError)?; let handle = usize::from_str(string).map_err(|_e| IPCError::ParseError)?; // SAFETY: This is a handle we passed in ourselves. let handle = unsafe { OwnedHandle::from_raw_handle(handle as RawHandle) }; - let event = create_manual_reset_event()?; + let event = create_manual_reset_event().map_err(IPCListenerError::CreationError)?; let mut listener = IPCListener { server_addr, @@ -129,7 +140,10 @@ impl IPCListener { // used internally and never visible externally. unsafe impl Send for IPCListener {} -fn create_named_pipe(server_addr: &CStr, first_instance: bool) -> Result<OwnedHandle, IPCError> { +fn create_named_pipe( + server_addr: &CStr, + first_instance: bool, +) -> Result<OwnedHandle, PlatformError> { const PIPE_BUFFER_SIZE: u32 = 4096; let open_mode = PIPE_ACCESS_DUPLEX @@ -162,7 +176,7 @@ fn create_named_pipe(server_addr: &CStr, first_instance: bool) -> Result<OwnedHa }; if pipe == INVALID_HANDLE_VALUE { - return Err(IPCError::System(unsafe { GetLastError() })); + return Err(PlatformError::CreatePipeFailure(unsafe { GetLastError() })); } // SAFETY: We just verified that the handle is valid. diff --git a/toolkit/crashreporter/crash_helper_common/src/lib.rs b/toolkit/crashreporter/crash_helper_common/src/lib.rs @@ -13,7 +13,7 @@ mod ipc_connector; mod ipc_listener; mod platform; -use errors::MessageError; +use messages::MessageError; // Re-export the platform-specific types and functions pub use crate::breakpad::{BreakpadChar, BreakpadData, BreakpadRawData, Pid}; @@ -21,8 +21,8 @@ pub use crate::ipc_channel::{IPCChannel, IPCClientChannel}; pub use crate::ipc_connector::{ AncillaryData, IPCConnector, IPCEvent, RawAncillaryData, INVALID_ANCILLARY_DATA, }; -pub use crate::ipc_listener::IPCListener; -pub use crate::platform::ProcessHandle; +pub use crate::ipc_listener::{IPCListener, IPCListenerError}; +pub use crate::platform::{PlatformError, ProcessHandle}; #[cfg(target_os = "windows")] pub use crate::platform::server_addr; diff --git a/toolkit/crashreporter/crash_helper_common/src/messages.rs b/toolkit/crashreporter/crash_helper_common/src/messages.rs @@ -7,13 +7,33 @@ use minidump_writer::minidump_writer::{AuxvType, DirectAuxvDumpInfo}; use num_derive::{FromPrimitive, ToPrimitive}; use num_traits::FromPrimitive; use std::{ - ffi::{CString, OsString}, + array::TryFromSliceError, + ffi::{CString, FromBytesWithNulError, NulError, OsString}, mem::size_of, }; +use thiserror::Error; #[cfg(target_os = "windows")] use windows_sys::Win32::System::Diagnostics::Debug::{CONTEXT, EXCEPTION_RECORD}; -use crate::{breakpad::Pid, errors::MessageError, ipc_connector::AncillaryData, BreakpadString}; +use crate::{breakpad::Pid, ipc_connector::AncillaryData, BreakpadString}; + +#[derive(Debug, Error)] +pub enum MessageError { + #[error("Nul terminator found within a string")] + InteriorNul(#[from] NulError), + #[error("The message contained an invalid payload")] + InvalidData, + #[error("Message kind is invalid")] + InvalidKind, + #[error("Invalid message size")] + InvalidSize(#[from] TryFromSliceError), + #[error("Missing ancillary data")] + MissingAncillary, + #[error("Missing nul terminator")] + MissingNul(#[from] FromBytesWithNulError), + #[error("Truncated message")] + Truncated, +} #[repr(u8)] #[derive(Copy, Clone, Debug, FromPrimitive, ToPrimitive, PartialEq)] @@ -606,9 +626,7 @@ pub struct RegisterChildProcess { impl RegisterChildProcess { pub fn new(ipc_endpoint: AncillaryData) -> RegisterChildProcess { - RegisterChildProcess { - ipc_endpoint, - } + RegisterChildProcess { ipc_endpoint } } fn payload_size(&self) -> usize { diff --git a/toolkit/crashreporter/crash_helper_common/src/platform.rs b/toolkit/crashreporter/crash_helper_common/src/platform.rs @@ -3,19 +3,19 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ #[cfg(target_os = "windows")] -pub use windows::{server_addr, ProcessHandle}; +pub use windows::{server_addr, PlatformError, ProcessHandle}; #[cfg(target_os = "windows")] pub(crate) mod windows; #[cfg(any(target_os = "android", target_os = "linux"))] -pub use linux::ProcessHandle; +pub use linux::{PlatformError, ProcessHandle}; #[cfg(any(target_os = "android", target_os = "linux"))] pub(crate) mod linux; #[cfg(target_os = "macos")] -pub use macos::ProcessHandle; +pub use macos::{PlatformError, ProcessHandle}; #[cfg(target_os = "macos")] pub(crate) mod macos; diff --git a/toolkit/crashreporter/crash_helper_common/src/platform/linux.rs b/toolkit/crashreporter/crash_helper_common/src/platform/linux.rs @@ -3,33 +3,61 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use nix::{ + errno::Errno, fcntl::{ fcntl, FcntlArg::{F_GETFL, F_SETFD, F_SETFL}, FdFlag, OFlag, }, sys::socket::{socketpair, AddressFamily, SockFlag, SockType}, - Result, }; use std::os::fd::{BorrowedFd, OwnedFd}; +use thiserror::Error; pub type ProcessHandle = (); -pub(crate) fn unix_socketpair() -> Result<(OwnedFd, OwnedFd)> { +#[derive(Error, Debug)] +pub enum PlatformError { + #[error("poll() call failed with error: {0}")] + PollFailure(Errno), + #[error("Could not set socket in non-blocking mode: {0}")] + SocketNonBlockError(Errno), + #[error("Could not flag socket as close-after-exec: {0}")] + SocketCloexecError(Errno), + #[error("Could not create a socket pair: {0}")] + SocketpairFailure(#[from] Errno), + #[error("sendmsg() call failed with error: {0}")] + SendFailure(Errno), + #[error("Sending {expected} bytes failed, only {sent} bytes sent")] + SendTooShort { expected: usize, sent: usize }, + #[error("recvmsg() call failed with error: {0}")] + ReceiveFailure(Errno), + #[error("Missing SCM credentials")] + ReceiveMissingCredentials, + #[error("Receiving {expected} bytes failed, only {received} bytes received")] + ReceiveTooShort { expected: usize, received: usize }, +} + +pub(crate) fn unix_socketpair() -> Result<(OwnedFd, OwnedFd), PlatformError> { socketpair( AddressFamily::Unix, SockType::SeqPacket, None, SockFlag::empty(), ) + .map_err(PlatformError::SocketpairFailure) } -pub(crate) fn set_socket_default_flags(socket: BorrowedFd) -> Result<()> { +pub(crate) fn set_socket_default_flags(socket: BorrowedFd) -> Result<(), PlatformError> { // All our sockets are in non-blocking mode. let flags = OFlag::from_bits_retain(fcntl(socket, F_GETFL)?); - fcntl(socket, F_SETFL(flags.union(OFlag::O_NONBLOCK))).map(|_res| ()) + fcntl(socket, F_SETFL(flags.union(OFlag::O_NONBLOCK))) + .map(|_res| ()) + .map_err(PlatformError::SocketNonBlockError) } -pub(crate) fn set_socket_cloexec(socket: BorrowedFd) -> Result<()> { - fcntl(socket, F_SETFD(FdFlag::FD_CLOEXEC)).map(|_res| ()) +pub(crate) fn set_socket_cloexec(socket: BorrowedFd) -> Result<(), PlatformError> { + fcntl(socket, F_SETFD(FdFlag::FD_CLOEXEC)) + .map(|_res| ()) + .map_err(PlatformError::SocketCloexecError) } diff --git a/toolkit/crashreporter/crash_helper_common/src/platform/macos.rs b/toolkit/crashreporter/crash_helper_common/src/platform/macos.rs @@ -11,25 +11,48 @@ use nix::{ }, libc::{setsockopt, SOL_SOCKET, SO_NOSIGPIPE}, sys::socket::{socketpair, AddressFamily, SockFlag, SockType}, - Result, }; use std::{ mem::size_of, os::fd::{AsRawFd, BorrowedFd, OwnedFd}, }; +use thiserror::Error; pub type ProcessHandle = (); -pub(crate) fn unix_socketpair() -> Result<(OwnedFd, OwnedFd)> { +#[derive(Error, Debug)] +pub enum PlatformError { + #[error("poll() call failed with error: {0}")] + PollFailure(Errno), + #[error("Could not set socket in non-blocking mode: {0}")] + SocketNonBlockError(Errno), + #[error("Could not flag socket as close-after-exec: {0}")] + SocketCloexecError(Errno), + #[error("Could not create a socket pair: {0}")] + SocketpairFailure(#[from] Errno), + #[error("sendmsg() call failed with error: {0}")] + SendFailure(Errno), + #[error("Sending {expected} bytes failed, only {sent} bytes sent")] + SendTooShort { expected: usize, sent: usize }, + #[error("recvmsg() call failed with error: {0}")] + ReceiveFailure(Errno), + #[error("Missing SCM credentials")] + ReceiveMissingCredentials, + #[error("Receiving {expected} bytes failed, only {received} bytes received")] + ReceiveTooShort { expected: usize, received: usize }, +} + +pub(crate) fn unix_socketpair() -> Result<(OwnedFd, OwnedFd), PlatformError> { socketpair( AddressFamily::Unix, SockType::Stream, None, SockFlag::empty(), ) + .map_err(PlatformError::SocketpairFailure) } -pub(crate) fn set_socket_default_flags(socket: BorrowedFd) -> Result<()> { +pub(crate) fn set_socket_default_flags(socket: BorrowedFd) -> Result<(), PlatformError> { // All our sockets are in non-blocking mode. let flags = OFlag::from_bits_retain(fcntl(socket, F_GETFL)?); fcntl(socket, F_SETFL(flags.union(OFlag::O_NONBLOCK)))?; @@ -48,12 +71,14 @@ pub(crate) fn set_socket_default_flags(socket: BorrowedFd) -> Result<()> { }; if res < 0 { - return Err(Errno::last()); + return Err(PlatformError::SocketNonBlockError(Errno::last())); } Ok(()) } -pub(crate) fn set_socket_cloexec(socket: BorrowedFd) -> Result<()> { - fcntl(socket, F_SETFD(FdFlag::FD_CLOEXEC)).map(|_res| ()) +pub(crate) fn set_socket_cloexec(socket: BorrowedFd) -> Result<(), PlatformError> { + fcntl(socket, F_SETFD(FdFlag::FD_CLOEXEC)) + .map(|_res| ()) + .map_err(PlatformError::SocketCloexecError) } diff --git a/toolkit/crashreporter/crash_helper_common/src/platform/windows.rs b/toolkit/crashreporter/crash_helper_common/src/platform/windows.rs @@ -2,10 +2,7 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::{ - errors::{IPCError, MessageError}, - Pid, IO_TIMEOUT, -}; +use crate::{Pid, IO_TIMEOUT}; use std::{ ffi::CString, mem::zeroed, @@ -13,6 +10,7 @@ use std::{ ptr::{null, null_mut}, rc::Rc, }; +use thiserror::Error; use windows_sys::Win32::{ Foundation::{ GetLastError, ERROR_IO_INCOMPLETE, ERROR_IO_PENDING, ERROR_NOT_FOUND, ERROR_PIPE_CONNECTED, @@ -28,6 +26,34 @@ use windows_sys::Win32::{ pub type ProcessHandle = OwnedHandle; +#[derive(Error, Debug)] +pub enum PlatformError { + #[error("Could not accept incoming connection: {0}")] + AcceptFailed(WIN32_ERROR), + #[error("Failed to duplicate clone handle")] + CloneHandleFailed(#[source] std::io::Error), + #[error("Could not create event: {0}")] + CreateEventFailed(WIN32_ERROR), + #[error("Could not create a pipe: {0}")] + CreatePipeFailure(WIN32_ERROR), + #[error("I/O error: {0}")] + IOError(WIN32_ERROR), + #[error("No process handle specified")] + MissingProcessHandle, + #[error("Could not listen for incoming connections: {0}")] + ListenFailed(WIN32_ERROR), + #[error("Receiving {expected} bytes failed, only {received} bytes received")] + ReceiveTooShort { expected: usize, received: usize }, + #[error("Could not reset event: {0}")] + ResetEventFailed(WIN32_ERROR), + #[error("Sending {expected} bytes failed, only {sent} bytes sent")] + SendTooShort { expected: usize, sent: usize }, + #[error("Could not set event: {0}")] + SetEventFailed(WIN32_ERROR), + #[error("Value too large")] + ValueTooLarge, +} + pub(crate) fn get_last_error() -> WIN32_ERROR { // SAFETY: This is always safe to call unsafe { GetLastError() } @@ -38,7 +64,7 @@ pub fn server_addr(pid: Pid) -> CString { CString::new(format!("\\\\.\\pipe\\gecko-crash-helper-pipe.{pid:}")).unwrap() } -pub(crate) fn create_manual_reset_event() -> Result<OwnedHandle, IPCError> { +pub(crate) fn create_manual_reset_event() -> Result<OwnedHandle, PlatformError> { // SAFETY: We pass null pointers for all the pointer arguments. let raw_handle = unsafe { CreateEventA( @@ -50,17 +76,26 @@ pub(crate) fn create_manual_reset_event() -> Result<OwnedHandle, IPCError> { } as RawHandle; if raw_handle.is_null() { - return Err(IPCError::System(get_last_error())); + return Err(PlatformError::CreateEventFailed(get_last_error())); } // SAFETY: We just verified that `raw_handle` is valid. Ok(unsafe { OwnedHandle::from_raw_handle(raw_handle) }) } -fn set_event(handle: HANDLE) -> Result<(), IPCError> { +fn set_event(handle: HANDLE) -> Result<(), PlatformError> { // SAFETY: This is always safe, even when passing an invalid handle. if unsafe { SetEvent(handle) } == FALSE { - Err(IPCError::System(get_last_error())) + Err(PlatformError::SetEventFailed(get_last_error())) + } else { + Ok(()) + } +} + +fn reset_event(handle: HANDLE) -> Result<(), PlatformError> { + // SAFETY: This is always safe, even when passing an invalid handle. + if unsafe { ResetEvent(handle) } == FALSE { + Err(PlatformError::ResetEventFailed(get_last_error())) } else { Ok(()) } @@ -108,10 +143,11 @@ enum OverlappedOperationType { } impl OverlappedOperation { + // Asynchronously listen for an incoming connection pub(crate) fn listen( handle: &Rc<OwnedHandle>, event: HANDLE, - ) -> Result<OverlappedOperation, IPCError> { + ) -> Result<OverlappedOperation, PlatformError> { let mut overlapped = Self::overlapped_with_event(event)?; // SAFETY: We guarantee that the handle and OVERLAPPED object are both @@ -123,7 +159,7 @@ impl OverlappedOperation { if res != FALSE { // According to Microsoft's documentation this should never happen, // we check out of an abundance of caution. - return Err(IPCError::System(error)); + return Err(PlatformError::ListenFailed(error)); } if error == ERROR_PIPE_CONNECTED { @@ -131,7 +167,7 @@ impl OverlappedOperation { // waiting on it will return immediately. set_event(event)?; } else if error != ERROR_IO_PENDING { - return Err(IPCError::System(error)); + return Err(PlatformError::ListenFailed(error)); } Ok(OverlappedOperation { @@ -141,7 +177,9 @@ impl OverlappedOperation { }) } - pub(crate) fn accept(mut self, handle: HANDLE) -> Result<(), IPCError> { + // Synchronously accept an incoming connection, does not wait and fails if + // no incoming connection is present. + pub(crate) fn accept(mut self, handle: HANDLE) -> Result<(), PlatformError> { let overlapped = self.overlapped.take().unwrap(); let mut _number_of_bytes_transferred: u32 = 0; // SAFETY: The pointer to the OVERLAPPED structure is under our @@ -163,7 +201,7 @@ impl OverlappedOperation { self.cancel_or_leak(overlapped, None); } - return Err(IPCError::System(error)); + return Err(PlatformError::AcceptFailed(error)); } Ok(()) @@ -173,7 +211,7 @@ impl OverlappedOperation { mut self, optype: OverlappedOperationType, wait: bool, - ) -> Result<Option<Vec<u8>>, IPCError> { + ) -> Result<Option<Vec<u8>>, PlatformError> { let overlapped = self.overlapped.take().unwrap(); let buffer = self.buffer.take().unwrap(); let mut number_of_bytes_transferred: u32 = 0; @@ -196,11 +234,20 @@ impl OverlappedOperation { self.cancel_or_leak(overlapped, Some(buffer)); } - return Err(IPCError::System(error)); + return Err(PlatformError::IOError(error)); } - if (number_of_bytes_transferred as usize) != buffer.len() { - return Err(IPCError::BadMessage(MessageError::InvalidData)); + if number_of_bytes_transferred as usize != buffer.len() { + return Err(match optype { + OverlappedOperationType::Read => PlatformError::ReceiveTooShort { + expected: buffer.len(), + received: number_of_bytes_transferred as usize, + }, + OverlappedOperationType::Write => PlatformError::SendTooShort { + expected: buffer.len(), + sent: number_of_bytes_transferred as usize, + }, + }); } Ok(match optype { @@ -213,10 +260,12 @@ impl OverlappedOperation { handle: &Rc<OwnedHandle>, event: HANDLE, expected_size: usize, - ) -> Result<OverlappedOperation, IPCError> { + ) -> Result<OverlappedOperation, PlatformError> { let mut overlapped = Self::overlapped_with_event(event)?; let mut buffer = vec![0u8; expected_size]; - let number_of_bytes_to_read: u32 = expected_size.try_into()?; + let number_of_bytes_to_read: u32 = expected_size + .try_into() + .map_err(|_e| PlatformError::ValueTooLarge)?; // SAFETY: We control all the pointers going into this call, guarantee // that they're valid and that they will be alive for the entire // duration of the asynchronous operation. @@ -236,7 +285,7 @@ impl OverlappedOperation { // waiting on it will return immediately. set_event(event)?; } else if error != ERROR_IO_PENDING { - return Err(IPCError::System(error)); + return Err(PlatformError::IOError(error)); } Ok(OverlappedOperation { @@ -246,7 +295,7 @@ impl OverlappedOperation { }) } - pub(crate) fn collect_recv(self, wait: bool) -> Result<Vec<u8>, IPCError> { + pub(crate) fn collect_recv(self, wait: bool) -> Result<Vec<u8>, PlatformError> { Ok(self.await_io(OverlappedOperationType::Read, wait)?.unwrap()) } @@ -254,9 +303,12 @@ impl OverlappedOperation { handle: &Rc<OwnedHandle>, event: HANDLE, mut buffer: Vec<u8>, - ) -> Result<OverlappedOperation, IPCError> { + ) -> Result<OverlappedOperation, PlatformError> { let mut overlapped = Self::overlapped_with_event(event)?; - let number_of_bytes_to_write: u32 = buffer.len().try_into()?; + let number_of_bytes_to_write: u32 = buffer + .len() + .try_into() + .map_err(|_e| PlatformError::ValueTooLarge)?; // SAFETY: We control all the pointers going into this call, guarantee // that they're valid and that they will be alive for the entire // duration of the asynchronous operation. @@ -276,7 +328,7 @@ impl OverlappedOperation { // waiting on it will return immediately. set_event(event)?; } else if error != ERROR_IO_PENDING { - return Err(IPCError::System(error)); + return Err(PlatformError::IOError(error)); } Ok(OverlappedOperation { @@ -286,16 +338,13 @@ impl OverlappedOperation { }) } - pub(crate) fn complete_send(self, wait: bool) -> Result<(), IPCError> { + pub(crate) fn complete_send(self, wait: bool) -> Result<(), PlatformError> { self.await_io(OverlappedOperationType::Write, wait)?; Ok(()) } - fn overlapped_with_event(event: HANDLE) -> Result<Box<OVERLAPPED>, IPCError> { - // SAFETY: This is always safe, even when passing an invalid handle. - if unsafe { ResetEvent(event) } == FALSE { - return Err(IPCError::System(get_last_error())); - } + fn overlapped_with_event(event: HANDLE) -> Result<Box<OVERLAPPED>, PlatformError> { + reset_event(event)?; Ok(Box::new(OVERLAPPED { hEvent: event, diff --git a/toolkit/crashreporter/crash_helper_server/src/ipc_server/unix.rs b/toolkit/crashreporter/crash_helper_server/src/ipc_server/unix.rs @@ -17,8 +17,8 @@ impl IPCServer { ); let mut events = Vec::<IPCEvent>::new(); - let mut num_events = - ignore_eintr!(poll(&mut pollfds, PollTimeout::NONE)).map_err(IPCError::System)?; + let mut num_events = ignore_eintr!(poll(&mut pollfds, PollTimeout::NONE)) + .map_err(|errno| IPCError::WaitingFailure(Some(errno)))?; for (index, pollfd) in pollfds.iter().enumerate() { // revents() returns None only if the kernel sends back data diff --git a/toolkit/crashreporter/crash_helper_server/src/ipc_server/windows.rs b/toolkit/crashreporter/crash_helper_server/src/ipc_server/windows.rs @@ -4,7 +4,7 @@ use std::convert::TryInto; -use crash_helper_common::{errors::IPCError, IPCEvent}; +use crash_helper_common::{errors::IPCError, IPCEvent, PlatformError}; use log::error; use windows_sys::Win32::{ Foundation::{ERROR_BROKEN_PIPE, FALSE, HANDLE, WAIT_OBJECT_0}, @@ -60,7 +60,9 @@ impl IPCServer { events.push(IPCEvent::Header(index, header)); } Err(error) => match error { - IPCError::System(_code @ ERROR_BROKEN_PIPE) => { + IPCError::ReceptionFailure( + _error @ PlatformError::IOError(ERROR_BROKEN_PIPE), + ) => { events.push(IPCEvent::Disconnect(index)); } _ => return Err(error),