Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Time-of-Check to Time-of-Use (TOCTOU) vulnerability parsing message queue headers from coherent DMA memory. -- commit e9f51c49f1590127c704379d2f179c17f740df94 Author: SeungJong Ha <[email protected]> gpu: nova-core: gsp: convert GspMem to zerocopy via the transmute bridge This commit converts GspMem and related structures to use zerocopy for safe transmutations by adding explicit padding and deriving FromBytes and IntoBytes. It replaces the manual unsafe transmute implementations. > diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs > b/drivers/gpu/nova-core/gsp/cmdq.rs > index 070de0731e950..d9929d24d3165 100644 > --- a/drivers/gpu/nova-core/gsp/cmdq.rs > +++ b/drivers/gpu/nova-core/gsp/cmdq.rs [ ... ] > @@ -169,6 +169,7 @@ struct MsgqData { > /// read pointer of `rx` actually refers to the `Msgq` owned by the other > side. > /// This design ensures that only the driver or GSP ever writes to a given > instance of this struct. > #[repr(C)] > +#[derive(FromBytes, IntoBytes, Immutable)] > // There is no struct defined for this in the open-gpu-kernel-source headers. > // Instead it is defined by code in `GspMsgQueuesInit()`. > // TODO: Revert to private once `IoView` projections replace the `gsp_mem` > module. [Severity: High] This isn't a bug introduced by this patch, but could this coherent shared memory lead to a time-of-check to time-of-use vulnerability? The driver validates lengths and checksums by reading fields like length from GspMsgElement, which is mapped directly into shared memory. For instance, in wait_for_msg(): wait_for_msg() let (header, slice_1) = GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?; However, receive_msg() seems to re-read the header fields directly from shared memory to advance the ring buffer pointer: receive_msg() self.gsp_mem.advance_cpu_read_ptr(u32::try_from( message.header.length().div_ceil(GSP_PAGE_SIZE), )?); Can a compromised hardware component modify the message length concurrently after the initial validation but before pointer advancement, potentially corrupting the host read pointer? Similarly, send_single_command() initializes a message header in shared memory and then reads its element_count to advance the write pointer: send_single_command() let elem_count = dst.header.element_count(); self.seq += 1; self.gsp_mem.advance_cpu_write_ptr(elem_count); Does this allow the device to race and corrupt the host write pointer by modifying element_count before it is read back? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
