Re: [PATCH 05/10] gpu: nova-core: gsp: cmdq: split the transport part of the send path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpu: nova-core: spurious checksum failure due to un-truncated slices
- [High] gpu: nova-core: infinite loop via unvalidated element_count from GSP
--

commit 90b2987a6783eb2acf6f7b5e9bf5e7fbc3108e1d
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: cmdq: split the transport part of the send path
    
Move the transport part of send_single_command into send_command_element.
The RPC part of send_single_command is passed as a closure to set up moving 
the RPC code into its own sub-module. No functional change intended.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index a5595da23407b..602824236f64e 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs

[ ... ]

> @@ -768,16 +807,28 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>          // Extract the `GspMsgElement`.
>          let (header, slice_1) = GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?;
>  
> +        // Validate checksum.
> +        if Cmdq::calculate_checksum(SBufferIter::new_reader([
> +            header.as_bytes(),
> +            slice_1,
> +            slice_2,
> +        ])) != 0
> +        {

[Severity: High]
Does this code spuriously fail the checksum validation when the GSP leaves
padding in the ring buffer or batches messages?

The patch moves the Cmdq::calculate_checksum call in wait_for_msg before
slice_1 and slice_2 are truncated to the actual payload_length. Because
calculate_checksum iterates and hashes all bytes in the slices, it will now
hash the entire available read area (including trailing ring buffer data)
instead of just the single message.

> +            dev_err!(&self.dev, "GSP receive: bad checksum\n");
> +            return Err(EIO);
> +        }

[ ... ]

> @@ -863,9 +899,8 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
>          };
>  
>          // Advance the read pointer past this message.
> -        self.gsp_mem.advance_cpu_read_ptr(u32::try_from(
> -            message.header.length().div_ceil(GSP_PAGE_SIZE),
> -        )?);
> +        self.gsp_mem
> +            .advance_cpu_read_ptr(message.header.element_count());

[Severity: High]
Can an untrusted or erroneously crafted message header from the GSP cause an
infinite loop here?

The patch changes the read pointer advancement in receive_msg to use
message.header.element_count() (the raw elemCount field from the GSP) instead
of safely deriving it from the bounded payload length.

If the GSP firmware provides an elemCount of 0 (or a multiple of
MSGQ_NUM_PAGES, i.e., 63), advance_cpu_read_ptr adds 0 to the read pointer.
If the driver then encounters an unexpected function code and retries, it
will continuously re-read the exact same zero-advancing message, resulting
in a deadlock.

>  
>          result
>      }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260927-cmdq-rpc-v1-0-822db5af910e@nvidia.com?part=5