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 <[email protected]> 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/[email protected]?part=1
