Re: [PATCH v3 1/9] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpu: nova-core: TOCTOU race in GSP message checksum validation allows bypassing integrity checks
--

commit 7f9f9f297ff07c9a2fc40d814e38057d895893f1
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: cmdq: validate checksum earlier on receive

The checksum validation error message of wait_for_msg() prints the
message's sequence number before validating the checksum. But the
checksum is part of the transport layer, and it not validating indicates
a corruption. Thus, this commit moves the checksum validation before
we start looking at the message header, and drops the sequence number
from the error message.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index a5595da23407b..d293d28b09674 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -768,6 +768,15 @@ 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 after truncating the message to its exact length.
> +        if Cmdq::calculate_checksum(
> +            SBufferIter::new_reader([header.as_bytes(), slice_1, slice_2]).take(header.length()),

[Severity: High]
Does this regression create a TOCTOU race condition in wait_for_msg()?
header appears to map directly to DMA memory, and header.length() is
read here for the first time to determine the boundaries for the checksum
validation.

If the GSP (or a hardware glitch) modifies the length field in DMA to a
larger value after this checksum passes, it is read again further down in
wait_for_msg():

        let payload_length = header.payload_length();

        // Check that the driver read area is large enough for the message.
        if slice_1.len() + slice_2.len() < payload_length {
            return Err(EIO);
        }

Could this secondary read use the modified length, causing the slices to be
truncated using the corrupted length and bypassing the integrity check for
the unverified data passed to the RPC handler?

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

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-cmdq-rpc-v3-0-91613f06520b@nvidia.com?part=1