Hi Alex, > On Dec 15, 2025, at 9:57 PM, Alexandre Courbot <[email protected]> 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. > > Fixes: 75f6b1de8133 ("gpu: nova-core: gsp: Add GSP command queue bindings and > handling") > Reviewed-by: Lyude Paul <[email protected]> > Signed-off-by: Alexandre Courbot <[email protected]> > --- > 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,
The panic comments also need updating? Other than the nit, Reviewed-by: Joel Fernandes <[email protected]> thanks, - Joel > ) > }; > > 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 >
