Re: [PATCH 1/2] gpu: nova-core: use kernel lossless integer conversion module

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A TOCTOU double-fetch vulnerability in GSP message parsing allows compromised or malicious GSP firmware to corrupt the CPU's message queue read pointer, causing desynchronization and potential denial of service.
--

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

gpu: nova-core: use kernel lossless integer conversion module

The kernel crate now features a replacement for our lossless integer
conversion routines. Switch to the kernel version and remove our own.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 6da728201281b..f85fde09aa6e3 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs

[ ... ]

> @@ -162,7 +162,7 @@ fn read(
>  #[repr(C, align(0x1000))]
>  #[derive(Debug)]
>  struct MsgqData {
> -    data: [[u8; GSP_PAGE_SIZE]; num::u32_as_usize(MSGQ_NUM_PAGES)],
> +    data: [[u8; GSP_PAGE_SIZE]; casts::u32_as_usize(MSGQ_NUM_PAGES)],
>  }

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing cmdq.rs, I
noticed a pre-existing double-fetch issue that could allow a compromised GSP
firmware to desynchronize the message queue.

Could the GSP modify the message length in DMA memory after it has been
validated, causing the host to corrupt its CPU read pointer?

In CmdqInner::wait_for_msg(), the message length is fetched from DMA memory
and validated against the available buffer size to ensure it is safe:

CmdqInner::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);
    }
    ...
}

However, upon returning to CmdqInner::receive_msg(), the length is fetched
a second time directly from the shared memory when advancing the queue read
pointer:

CmdqInner::receive_msg() {
    ...
    let message = self.wait_for_msg(timeout)?;
    ...
    // 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),
    )?);
    ...
}

Because message.header is a reference into DMA memory, this second fetch
uses an unvalidated value. If the GSP modifies the length field in DMA
memory between these two reads, the kernel will advance its read pointer by
an unvalidated amount. This could cause the kernel to misinterpret queue
memory, fail subsequent validations, and lose communication with the GSP.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828-nova_num-v1-0-e21f17ba4127@nvidia.com?part=1