On Mon Jun 29, 2026 at 4:10 PM JST, Alexandre Courbot wrote:
> On Mon Jun 29, 2026 at 3:21 AM JST, SeungJong Ha wrote:
>> On Sun Jun 28, 2026 at 5:22 PM UTC, Sashiko AI review wrote:
>>> This isn't a bug introduced by this patch, but could this coherent shared
>>> memory lead to a time-of-check to time-of-use vulnerability?
>>>
>>> The driver validates lengths and checksums by reading fields like length
>>> from GspMsgElement, which is mapped directly into shared memory. For
>>> instance, in wait_for_msg():
>>>
>>> wait_for_msg()
>>>     let (header, slice_1) = 
>>> GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?;
>>>
>>> However, receive_msg() seems to re-read the header fields directly from
>>> shared memory to advance the ring buffer pointer:
>>>
>>> receive_msg()
>>>     self.gsp_mem.advance_cpu_read_ptr(u32::try_from(
>>>         message.header.length().div_ceil(GSP_PAGE_SIZE),
>>>     )?);
>>>
>>> Can a compromised hardware component modify the message length concurrently
>>> after the initial validation but before pointer advancement, potentially
>>> corrupting the host read pointer?
>>>
>>> Similarly, send_single_command() initializes a message header in shared
>>> memory and then reads its element_count to advance the write pointer:
>>>
>>> send_single_command()
>>>     let elem_count = dst.header.element_count();
>>>     self.seq += 1;
>>>     self.gsp_mem.advance_cpu_write_ptr(elem_count);
>>>
>>> Does this allow the device to race and corrupt the host write pointer by
>>> modifying element_count before it is read back?
>>
>> This is pre-existing and not changed by this patch: it only makes
>> explicit (via a checked `zerocopy` derive) what the previous `unsafe
>> impl transmute::{FromBytes, AsBytes}` already allowed implicitly -- the
>> layout is byte-identical and the message-handling path is untouched -- so
>> it neither introduces nor addresses this. I'm not familiar enough with
>> the GSP threat model to judge whether the TOCTOU is in scope here; if it
>> is worth noting, I can add a TODO comment near the affected reads.
>
> So I understand that this as a copy-pasted Claude/Sashiko block, but
> would also appreciate if the human behind the keyboard could provide the
> context required to easily understand which part of the code this is
> about.

Sorry, that reply was an unedited block. Here is the concrete context.

It is the message-queue read path in gsp/cmdq.rs (wait_for_msg() /
receive_msg() / send_single_command()).
I haven't touched that logic; this patch only swaps the unsafe transmute
impls for a checked zerocopy derive. If it's worth noting, I'm happy to add
a comment near those reads.

Thanks,
SeungJong

Reply via email to