linux-kernel - Re: [PATCH v3 2/4] gpu: nova-core: gsp: fix length of received messages

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <hbo3azmt6mpaq7dyn4mnitrg3iouk3eeijjqinpl4grlufrvqr@6ieka4obgszr>
Date: Tue, 16 Dec 2025 17:14:15 +1100
From: Alistair Popple <apopple@...dia.com>
To: Alexandre Courbot <acourbot@...dia.com>
Cc: Danilo Krummrich <dakr@...nel.org>, Alice Ryhl <aliceryhl@...gle.com>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, Benno Lossin <lossin@...nel.org>, 
	Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>, 
	John Hubbard <jhubbard@...dia.com>, Joel Fernandes <joelagnelf@...dia.com>, 
	Timur Tabi <ttabi@...dia.com>, Edwin Peer <epeer@...dia.com>, nouveau@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org, 
	Lyude Paul <lyude@...hat.com>
Subject: Re: [PATCH v3 2/4] gpu: nova-core: gsp: fix length of received
 messages

On 2025-12-16 at 13:57 +1100, Alexandre Courbot <acourbot@...dia.com> wrote...
> The size of messages' payload is miscalculated, leading to extra data
> passed to the message handler. While this is not a problem with our
> current set of commands, others with a variable-length payload may
> misbehave. Fix this by introducing a method returning the payload size
> and using it.

The whole inconsistency of the message element struct not including it's header
fields in the size whilst the rpc struct does has caused endless confusion, this
looks much better, thanks for fixing!

Reviewed-by: Alistair Popple <apopple@...dia.com>

> Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and handling")
> Reviewed-by: Lyude Paul <lyude@...hat.com>
> Signed-off-by: Alexandre Courbot <acourbot@...dia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 10 ++++++----
>  drivers/gpu/nova-core/gsp/fw.rs   | 13 +++++++++----
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 6f946d14868a..7985a0b3f769 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -588,21 +588,23 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>              header.length(),
>          );
>  
> +        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() < header.length() {
> +        if slice_1.len() + slice_2.len() < payload_length {
>              return Err(EIO);
>          }
>  
>          // Cut the message slices down to the actual length of the message.
> -        let (slice_1, slice_2) = if slice_1.len() > header.length() {
> +        let (slice_1, slice_2) = if slice_1.len() > payload_length {
>              // PANIC: we checked above that `slice_1` is at least as long as `msg_header.length()`.
> -            (slice_1.split_at(header.length()).0, &slice_2[0..0])
> +            (slice_1.split_at(payload_length).0, &slice_2[0..0])
>          } else {
>              (
>                  slice_1,
>                  // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
>                  // large as `msg_header.length()`.
> -                slice_2.split_at(header.length() - slice_1.len()).0,
> +                slice_2.split_at(payload_length - slice_1.len()).0,
>              )
>          };
>  
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index abffd6beec65..7b8e710b33e7 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -853,11 +853,16 @@ pub(crate) fn set_checksum(&mut self, checksum: u32) {
>          self.inner.checkSum = checksum;
>      }
>  
> -    /// Returns the total length of the message.
> +    /// Returns the length of the message's payload.
> +    pub(crate) fn payload_length(&self) -> usize {
> +        // `rpc.length` includes the length of the RPC message header.
> +        num::u32_as_usize(self.inner.rpc.length)
> +            .saturating_sub(size_of::<bindings::rpc_message_header_v>())
> +    }
> +
> +    /// Returns the total length of the message, message and RPC headers included.
>      pub(crate) fn length(&self) -> usize {
> -        // `rpc.length` includes the length of the GspRpcHeader but not the message header.
> -        size_of::<Self>() - size_of::<bindings::rpc_message_header_v>()
> -            + num::u32_as_usize(self.inner.rpc.length)
> +        size_of::<Self>() + self.payload_length()
>      }
>  
>      // Returns the sequence number of the message.
> 
> -- 
> 2.52.0
>