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