Re: How to fix problems with the sashiko review model
James Bottomley <[email protected]> Wed, 22 Jul 2026 09:42:46 -0400
| Newsgroups | dev.linux.lists.ksummit,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-scsi |
|---|---|
| Message-ID | <e8f7c3614c15a86b335576074fec1768d95f3df4.camel@HansenPartnership.com> |
On Wed, 2026-07-22 at 15:08 +0200, 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.=C2=A0 We got this review in SCSI on > > this thread: > >=20 > > https://lore.kernel.org/linux-scsi/20260722041257.1557-1-pengpeng@iscas= .ac.cn/ > >=20 > > But this part of the review > >=20 > > On Wed, 2026-07-22 at 04:27 +0000, [email protected]=C2=A0wrote: > > [...] > > > > + > > > > + reply_len =3D mpi_reply->MsgLength * 4; > > > > + if (reply_len < event_offset || reply_len > ioc- > > > > >reply_sz) > > > > + return false; > > > > + > > > > + event_data_len =3D le16_to_cpu(mpi_reply- > > > > >EventDataLength) * > > > > 4; > > >=20 > > > [Severity: High] > > > Is there a Time-of-Check to Time-of-Use window here since > > > mpi_reply is read directly from shared DMA memory? > > >=20 > > > 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. > > >=20 > > > Could a malicious device mutate these fields in the window > > > between validation and use to bypass the checks introduced by > > > this patch? > >=20 > > Is wrong.=C2=A0 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).=C2=A0 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). > >=20 > > 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: > >=20 > > https://github.com/masoncl/review-prompts > >=20 > > And what I need to do is to add an additional drivers.md file to > > kernel/subsystems explaining this?=C2=A0 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. >=20 > Please make this per-subsystem, as your description above doesn't > hold for all types of devices or busses.=C2=A0 See this thread where I > describe what we do for USB and asking for much the same type of > description for the virtio drivers: > =09 > https://lore.kernel.org/all/2026072036-outburst-rebel-c71b@gregkh/ Well, I noted that in my reply above. The way I was thinking of implementing it was to add a general instruction file for drivers which would make hardware trusted for pretty much everything and then instruct the AI to consult driver specific files for overrides to this so we could add the additional threat checks to usb.md and virt.md Regards, James