Re: How to fix problems with the sashiko review model

Greg KH <[email protected]> Wed, 22 Jul 2026 15:08:44 +0200
Newsgroups dev.linux.lists.ksummit,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-scsi
Message-ID <2026072224-moisture-gonad-409f@gregkh>
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/

thanks,

greg k-h