Linux SCSI subsystem development
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: sashiko-reviews@lists.linux.dev, Chris Mason <clm@fb.com>,
	Roman Gushchin <roman.gushchin@linux.dev>,
	linux-scsi@vger.kernel.org, ksummit@lists.linux.dev
Subject: Re: How to fix problems with the sashiko review model
Date: Wed, 22 Jul 2026 15:08:44 +0200	[thread overview]
Message-ID: <2026072224-moisture-gonad-409f@gregkh> (raw)
In-Reply-To: <dcadf40a07a616de931c484bbe68f4b3f1f83b80.camel@HansenPartnership.com>

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/20260722041257.1557-1-pengpeng@iscas.ac.cn/
> 
> But this part of the review
> 
> On Wed, 2026-07-22 at 04:27 +0000, sashiko-bot@kernel.org 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

  parent reply	other threads:[~2026-07-22 13:08 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22  4:12 [PATCH] scsi: mpt3sas: validate variable event array spans Pengpeng Hou
2026-07-22  4:27 ` sashiko-bot
2026-07-22 11:58   ` How to fix problems with the sashiko review model James Bottomley
2026-07-22 12:11     ` Laurent Pinchart
2026-07-22 12:42       ` James Bottomley
2026-07-22 13:08     ` Greg KH [this message]
2026-07-22 13:10       ` Greg KH
2026-07-22 13:45         ` James Bottomley
2026-07-22 14:08           ` Theodore Tso
2026-07-22 15:29           ` Miguel Ojeda
2026-07-22 13:42       ` James Bottomley
2026-07-22 14:00         ` Johannes Berg
2026-07-22 14:17           ` James Bottomley
2026-07-22 14:25             ` Johannes Berg
2026-07-22 15:39               ` Steven Rostedt
2026-07-22 15:55                 ` James Bottomley
2026-07-22 16:36                   ` Johannes Berg
2026-07-22 15:09             ` Mauro Carvalho Chehab
2026-07-22 15:24               ` James Bottomley
2026-07-22 20:25                 ` Mauro Carvalho Chehab
2026-07-22 14:20           ` Mark Brown
2026-07-22 13:43       ` Guenter Roeck
2026-07-22 15:10         ` James Bottomley
2026-07-22 15:41           ` Miguel Ojeda
2026-07-22 15:54           ` Arnaldo Carvalho de Melo
2026-07-22 16:40           ` Roman Gushchin
2026-07-22 15:55     ` Roman Gushchin
2026-07-22 11:32 ` [PATCH] scsi: mpt3sas: validate variable event array spans James Bottomley

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2026072224-moisture-gonad-409f@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=clm@fb.com \
    --cc=ksummit@lists.linux.dev \
    --cc=linux-scsi@vger.kernel.org \
    --cc=roman.gushchin@linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox