Re: [PATCH RFC 4/4] gpu: nova-core: gsp: convert GspMem to zerocopy via the transmute bridge

[sashiko-bot@kernel.org]

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 <engineer.jjhama@gmail.com>

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/20260628-dma-zerocopy-bridge-v1-0-9a2895ebe30d@gmail.com?part=4