Verified Commit e3e84626 authored by ma1's avatar ma1
Browse files

Bug 2068421 - Make crash helper message reception more robust on Windows

parent d2f340fb
Loading
Loading
Loading
Loading
+2 −0
Changes for toolkit/crashreporter/crash_helper_common/src/ipc_queue.rs: 2 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -30,6 +30,8 @@ pub enum IPCQueueError {
    IPCListenerError(#[from] IPCListenerError),
    #[error("Underlying message error: {0}")]
    MessageError(#[from] MessageError),
    #[error("Unexpected event: {0}")]
    UnexpectedEvent(&'static str),
}

/*****************************************************************************
+33 −22
Changes for toolkit/crashreporter/crash_helper_common/src/ipc_queue/windows.rs: 33 added lines, 22 removed lines.
Original line number Diff line number Diff line
@@ -181,27 +181,8 @@ impl IPCQueue {
                )
            };

            if number_of_bytes_transferred == 0 {
                // This is an event on the listener
                debug_assert!(
                    self.listener.as_raw() as IPCConnectorKey == completion_key,
                    "Completion event doesn't match the listener"
                );
                let operation = self.listen_operation.take();
                if let Some(operation) = operation {
                    operation
                        .accept()
                        .map_err(|_e| IPCQueueError::RegistrationFailure(0))?;
                }
                let connector = Rc::new(self.listener.replace_pipe()?);
                self.insert_connector(&connector);

                // After the pipe is connected the listener handle will have been
                // replaced with a new one, so associate the new handle with the
                // completion queue.
                self.add_handle(self.listener.as_raw())?;

                events.push(IPCEvent::Connect(connector));
            if self.is_listener(completion_key) {
                events.push(self.handle_listener_event(number_of_bytes_transferred)?);
            } else {
                let element = self
                    .connectors
@@ -211,7 +192,7 @@ impl IPCQueue {
                    .operation
                    .take()
                    .expect("No pending receive operation");
                let buffer = &operation.collect_recv();
                let buffer = operation.collect_recv(number_of_bytes_transferred as usize);
                let header = Header::decode(buffer)?;
                let payload = element.connector.recv(header.size);
                match payload {
@@ -235,6 +216,36 @@ impl IPCQueue {

        Ok(events)
    }

    fn is_listener(&self, completion_key: IPCConnectorKey) -> bool {
        self.listener.as_raw() as IPCConnectorKey == completion_key
    }

    fn handle_listener_event(
        &mut self,
        number_of_bytes_transferred: u32,
    ) -> Result<IPCEvent, IPCQueueError> {
        // A connection event will have 0 bytes transferred
        if number_of_bytes_transferred != 0 {
            return Err(IPCQueueError::UnexpectedEvent(
                "Non-zero transfer received on the listener",
            ));
        }

        let operation = self.listen_operation.take();
        if let Some(operation) = operation {
            operation.accept().map_err(|_e| IPCQueueError::RegistrationFailure(0))?;
        }
        let connector = Rc::new(self.listener.replace_pipe()?);
        self.insert_connector(&connector);

        // After the pipe is connected the listener handle will have been
        // replaced with a new one, so associate the new handle with the
        // completion queue.
        self.add_handle(self.listener.as_raw())?;

        Ok(IPCEvent::Connect(connector))
    }
}

impl Drop for IPCQueue {
+13 −2
Changes for toolkit/crashreporter/crash_helper_common/src/platform/windows.rs: 13 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -325,8 +325,19 @@ impl OverlappedOperation {
        Self::sched_recv_internal(handle, None, expected_size)
    }

    pub(crate) fn collect_recv(mut self) -> Vec<u8> {
        self.buffer.take().expect("Missing receive buffer")
    pub(crate) fn collect_recv(mut self, len: usize) -> Vec<u8> {
        let mut buffer = self.buffer.take().expect("Missing receive buffer");

        // We assert here because if this happens then we've hit a logic bug in
        // our code. The size of the receive operation and buffer length are
        // tied together so this should never happen.
        assert!(
            len <= buffer.len(),
            "More bytes received than the buffer would allow"
        );

        buffer.truncate(len);
        buffer
    }

    pub(crate) fn send(