Re: How to fix problems with the sashiko review model

Guenter Roeck <[email protected]> Wed, 22 Jul 2026 06:43:50 -0700
Newsgroups dev.linux.lists.ksummit,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-scsi
Message-ID <[email protected]>
On 7/22/26 06:08, Greg KH wrote:
> On Wed, Jul 22, 2026 at 07:58:48AM -0400, James Bottomley wrote:
>> There doesn't seem to be a Sashiko list, so I added ksummit as the most
>> likely one to catch attention.  We got this review in SCSI on this
>> thread:
>>
>> https://lore.kernel.org/linux-scsi/[email protected]/
>>
>> But this part of the review
>>
>> On Wed, 2026-07-22 at 04:27 +0000, [email protected] wrote:
>> [...]
>>>> +
>>>> +	reply_len = mpi_reply->MsgLength * 4;
>>>> +	if (reply_len < event_offset || reply_len > ioc->reply_sz)
>>>> +		return false;
>>>> +
>>>> +	event_data_len = le16_to_cpu(mpi_reply->EventDataLength) *
>>>> 4;
>>>
>>> [Severity: High]
>>> Is there a Time-of-Check to Time-of-Use window here since mpi_reply
>>> is read directly from shared DMA memory?
>>>
>>> The validation function reads bounds from mpi_reply, but after
>>> validation passes, mpt3sas_scsih_event_callback() re-reads fields
>>> like EventDataLength and NumEntries directly from the same DMA-backed
>>> memory.
>>>
>>> Could a malicious device mutate these fields in the window between
>>> validation and use to bypass the checks introduced by this patch?
>>
>> Is wrong.  Hardware may never be presumed to be malicious (well, except
>> if it's virtual hardware provided by an untrusted host to a
>> confidential guest, but that's a very small list of drivers or it's a
>> USB or thunderbolt device designed to attack or exfiltrate data from an
>> unattended system).  It may be buggy, but we never presume bugs until
>> someone finds a device exhibiting the bug in the field (so we don't
>> proactively fix them).
>>
>> The bigger part of this is that I was looking into how to fix this
>> (which might interest a broader swathe of developers than just SCSI).
>> I think the fault is in the review prompts:
>>
>> https://github.com/masoncl/review-prompts
>>
>> And what I need to do is to add an additional drivers.md file to
>> kernel/subsystems explaining this?  If that's right I can send a pull
>> request, but I thought I'd check here because the audience is much
>> bigger than if it were to get discussed in the github PR.
> 
> Please make this per-subsystem, as your description above doesn't hold
> for all types of devices or busses.  See this thread where I describe
> what we do for USB and asking for much the same type of description for
> the virtio drivers:
> 	https://lore.kernel.org/all/2026072036-outburst-rebel-c71b@gregkh/
> 

Agree.

For Sashiko, fork and clone https://github.com/sashiko-dev/sashiko.git,
submit your changes into the clone, and send a pull request. Or at least
that is how I do it.

In the Sashiko source, per-subsystem prompts are located in directory
third_party/prompts/kernel/subsystem.

Guenter