All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.