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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpu/nova-core: checksum validation fails on untrimmed queue buffer slices
- [High] gpu/nova-core: unvalidated element_count from GSP allows arbitrary read pointer manipulation
--

commit 4799dd94b859632b92a2a5821daacb47098f5faa
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: cmdq: split the transport part of the receive path

This commit splits wait_for_msg into wait_for_element and wait_for_msg. The
transport layer is moved into wait_for_element, while the RPC layer that
trims the payload slices remains in wait_for_msg. This prepares the code
for moving the RPC layer into its own module.

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

[ ... ]

> @@ -755,7 +584,7 @@ fn send_command<M>(&mut self, command: M) -> Result
>      ///   message queue.
>      ///
>      /// Error codes returned by the message constructor are propagated as-is.
> -    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
> +    fn wait_for_element(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>          // Wait for a message to arrive from the GSP.
>          let (slice_1, slice_2) = read_poll_timeout(
>              || Ok(self.gsp_mem.driver_read_area()),
> @@ -768,15 +597,41 @@ 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
> +        {
> +            dev_err!(&self.dev, "GSP receive: bad checksum\n");
> +            return Err(EIO);
> +        }
> +
> +        Ok(GspMessage {
> +            header,
> +            contents: (slice_1, slice_2),
> +        })
> +    }

[Severity: High]
Does this code calculate the checksum over the entire available read area,
including trailing padding or queued messages?

In the original wait_for_msg(), slice_1 and slice_2 were trimmed to the
message payload_length before the checksum calculation. Since
wait_for_element() performs validation before trimming, won't valid messages
fail the checksum if there's extra data in the buffer?

[ ... ]

> @@ -863,9 +703,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());
>  
>          result
>      }

[Severity: High]
Does this code allow an untrusted GSP to arbitrarily manipulate the host's
ring buffer read pointer?

The element_count() value comes directly from the GSP message header and
is never validated against the available read area or queue capacity, unlike
payload_length.

Can a malicious GSP send a small payload but a huge element_count to force
the kernel to process uninitialized memory or stale data?

(Note: In the final state of the patch series, this logic ends up in
receive_rpc_msg(), but the vulnerability is introduced here).

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