Re: How to fix problems with the sashiko review model

On Wed, Jul 22, 2026 at 03:08:44PM +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.  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/

Also, this should be in the in-kernel documentation so the LLMs in
general don't start sending us "fixes" for things that we don't want
declared as a security issue.
We've done this already for some things in the "threat model" file,
so perhaps put it there?   Then there's no need to add it to the github
repo.

thanks,

greg k-h