[PATCH] scsi: mpt3sas: validate variable event array spans

[PATCH] scsi: mpt3sas: validate variable event array spans

[Pengpeng Hou]

SAS topology, PCIe topology and IR configuration events contain flexible
arrays whose element counts are supplied by firmware. Their interrupt-time
handlers use NumEntries or NumElements without first checking that the
corresponding array fits in EventData.

Validate only these three variable-array event types before dispatch. Check
that MsgLength is within the allocated reply frame, that EventDataLength is
within MsgLength, and that the counted array fits in EventData.

Malformed events still follow the existing ACK path before they are
dropped, so validation does not leave an acknowledged event pending in
firmware. Other event types retain their existing behavior.

Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 71 +++++++++++++++++++++++++++++++++++--
 1 file changed, 69 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 79052f2accbd..2a9ea81d1e2b 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -1333,6 +1333,65 @@ _base_display_event_data(struct MPT3SAS_ADAPTER *ioc,
 	ioc_info(ioc, "%s\n", desc);
 }
 
+static bool
+_base_variable_event_data_valid(struct MPT3SAS_ADAPTER *ioc,
+				Mpi2EventNotificationReply_t *mpi_reply)
+{
+	const size_t event_offset = offsetof(Mpi2EventNotificationReply_t,
+					     EventData);
+	const void *event_data = mpi_reply->EventData;
+	size_t event_data_len;
+	size_t reply_len;
+	u16 event;
+
+	event = le16_to_cpu(mpi_reply->Event);
+	switch (event) {
+	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST:
+	case MPI2_EVENT_PCIE_TOPOLOGY_CHANGE_LIST:
+	case MPI2_EVENT_IR_CONFIGURATION_CHANGE_LIST:
+		break;
+	default:
+		return true;
+	}
+
+	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;
+	if (event_data_len > reply_len - event_offset)
+		return false;
+
+	switch (event) {
+	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST: {
+		const Mpi2EventDataSasTopologyChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi2EventDataSasTopologyChangeList_t,
+					    PHY);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, PHY, data->NumEntries) <= event_data_len;
+	}
+	case MPI2_EVENT_PCIE_TOPOLOGY_CHANGE_LIST: {
+		const Mpi26EventDataPCIeTopologyChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi26EventDataPCIeTopologyChangeList_t,
+					    PortEntry);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, PortEntry, data->NumEntries) <= event_data_len;
+	}
+	case MPI2_EVENT_IR_CONFIGURATION_CHANGE_LIST: {
+		const Mpi2EventDataIrConfigChangeList_t *data = event_data;
+		size_t fixed_len = offsetof(Mpi2EventDataIrConfigChangeList_t,
+					    ConfigElement);
+
+		return event_data_len >= fixed_len &&
+		       struct_size(data, ConfigElement, data->NumElements) <= event_data_len;
+	}
+	default:
+		return true;
+	}
+}
+
 /**
  * _base_sas_log_info - verbose translation of firmware log info
  * @ioc: per adapter object
@@ -1482,8 +1541,9 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 {
 	Mpi2EventNotificationReply_t *mpi_reply;
 	Mpi2EventAckRequest_t *ack_request;
-	u16 smid;
 	struct _event_ack_list *delayed_event_ack;
+	bool event_data_valid;
+	u16 smid;
 
 	mpi_reply = mpt3sas_base_get_reply_virt_addr(ioc, reply);
 	if (!mpi_reply)
@@ -1491,7 +1551,12 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	if (mpi_reply->Function != MPI2_FUNCTION_EVENT_NOTIFICATION)
 		return 1;
 
-	_base_display_event_data(ioc, mpi_reply);
+	event_data_valid = _base_variable_event_data_valid(ioc, mpi_reply);
+	if (event_data_valid)
+		_base_display_event_data(ioc, mpi_reply);
+	else
+		ioc_warn(ioc, "dropping malformed event 0x%04x\n",
+			 le16_to_cpu(mpi_reply->Event));
 
 	if (!(mpi_reply->AckRequired & MPI2_EVENT_NOTIFICATION_ACK_REQUIRED))
 		goto out;
@@ -1521,6 +1586,8 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
 	ioc->put_smid_default(ioc, smid);
 
  out:
+	if (!event_data_valid)
+		return 1;
 
 	/* scsih callback handler */
 	mpt3sas_scsih_event_callback(ioc, msix_index, reply);
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] scsi: mpt3sas: validate variable event array spans

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] TOCTOU vulnerability on shared DMA memory bypasses event validation

Pre-existing issues:
- [High] Missing bounds check for unhandled event types allows out-of-bounds read
- [High] Kernel panic in error path due to kfree() on a flexible array member
--

--- Patch [1]: [PATCH] scsi: mpt3sas: validate variable event array spans ---
commit 151ce14025a2487b9520990fe50767c45e6d12d5
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>

scsi: mpt3sas: validate variable event array spans

This patch adds validation for SAS topology, PCIe topology, and IR configuration
events to ensure variable arrays fit within EventData before dispatch.
Malformed events are dropped after being acknowledged, while other event types
retain their existing behavior.

> diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
> index 79052f2accbd3..2a9ea81d1e2b0 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_base.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
[ ... ]
> @@ -1333,6 +1333,65 @@ _base_display_event_data(struct MPT3SAS_ADAPTER *ioc,
>  	ioc_info(ioc, "%s\n", desc);
>  }
>  
> +static bool
> +_base_variable_event_data_valid(struct MPT3SAS_ADAPTER *ioc,
> +				Mpi2EventNotificationReply_t *mpi_reply)
> +{
> +	const size_t event_offset = offsetof(Mpi2EventNotificationReply_t,
> +					     EventData);
> +	const void *event_data = mpi_reply->EventData;
> +	size_t event_data_len;
> +	size_t reply_len;
> +	u16 event;
> +
> +	event = le16_to_cpu(mpi_reply->Event);
> +	switch (event) {
> +	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST:
> +	case MPI2_EVENT_PCIE_TOPOLOGY_CHANGE_LIST:
> +	case MPI2_EVENT_IR_CONFIGURATION_CHANGE_LIST:
> +		break;
> +	default:
> +		return true;
> +	}

[Severity: High]
This is a pre-existing issue, but since we are adding validation here, does
returning true early leave other event types vulnerable to out-of-bounds
reads?

When mpt3sas_scsih_event_callback() processes unhandled events, it uses the
unvalidated EventDataLength to calculate the payload size:

    sz = le16_to_cpu(mpi_reply->EventDataLength) * 4;
    fw_event = alloc_fw_event_work(sz);
    ...
    memcpy(fw_event->event_data, mpi_reply->EventData, sz);

Could a compromised device supply a large EventDataLength (e.g. 0xFFFF) for
other events, causing memcpy() to read out-of-bounds from the ioc->reply
DMA pool?

> +
> +	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?

> +	if (event_data_len > reply_len - event_offset)
> +		return false;
> +
> +	switch (event) {
> +	case MPI2_EVENT_SAS_TOPOLOGY_CHANGE_LIST: {
[ ... ]
> @@ -1521,6 +1586,8 @@ _base_async_event(struct MPT3SAS_ADAPTER *ioc, u8 msix_index, u32 reply)
>  	ioc->put_smid_default(ioc, smid);
>  
>   out:
> +	if (!event_data_valid)
> +		return 1;
>  
>  	/* scsih callback handler */
>  	mpt3sas_scsih_event_callback(ioc, msix_index, reply);

[Severity: High]
This isn't a bug introduced by this patch, but looking at
mpt3sas_scsih_event_callback(), is it safe to call kfree() on a flexible
array member in the error path?

If kzalloc() fails for fw_event->retries, the error handler does:

    ioc_err(ioc, "failure!\n");
    kfree(fw_event->event_data);
    fw_event_work_put(fw_event);
    return 1;

Since event_data is declared as a flexible array member at the end of
struct fw_event_work, doesn't passing this interior pointer to kfree()
trigger a kernel panic?  The subsequent fw_event_work_put() already
frees the whole structure.

>  
>  	/* ctl callback handler */
>  	mpt3sas_ctl_event_callback(ioc, msix_index, reply);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260722041257.1557-1-pengpeng@iscas.ac.cn?part=1

How to fix problems with the sashiko review model

[James Bottomley]

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.

Regards,

James

Re: How to fix problems with the sashiko review model

[Laurent Pinchart]

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:

workflows@vger.kernel.org seems a good candidate for discussions about
tooling. The mailing list was created to share workflows and tools
between maintainers as far as I understand.

> 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.

-- 
Regards,

Laurent Pinchart

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 15:11 +0300, Laurent Pinchart 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:
> 
> workflows@vger.kernel.org seems a good candidate for discussions
> about tooling. The mailing list was created to share workflows and
> tools between maintainers as far as I understand.

Happy to keep this list cc'd, but I note I'm not on it so I'm not sure
how many other maintainers would be and the object is to engage driver
maintainer feedback on the proposal.

Regards,

James

Re: How to fix problems with the sashiko review model

[Greg KH]

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

Re: How to fix problems with the sashiko review model

[Greg KH]

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/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/

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

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 15:10 +0200, Greg KH wrote:
[...]
> 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.

So you're thinking of this file:

Documentation/process/threat-model.rst

?  It's already mostly in there, it's just that sashiko doesn't seem to
be picking it up (otherwise it wouldn't have suggested the problem I
noted), which is why I was thinking it needed to be a direct prompt ...
unless the prompt should be to read that file?

Regards,

James

Re: How to fix problems with the sashiko review model

[Theodore Tso]

On Wed, Jul 22, 2026 at 09:45:48AM -0500, James Bottomley wrote:
> So you're thinking of this file:
> 
> Documentation/process/threat-model.rst
> 
> ?  It's already mostly in there, it's just that sashiko doesn't seem to
> be picking it up (otherwise it wouldn't have suggested the problem I
> noted), which is why I was thinking it needed to be a direct prompt ...
> unless the prompt should be to read that file?

The threat-model.md is relatively new (May 2026), and there are
various security people who have asserted on linux-kernel that we need
to be afraid of malicious hardware.

I'll note that I and other file system developers have asserted that
mounting malicious corrupted file system images is not in our threat
model (and if you want it to be part of the threat model then fund the
engineers; if it doesn't impact performances, patches gratefully
accepted).  And it yet it took **years** to convince the Syzbot
security folks that this was something We Didn't Care About and to
please stop generating the noise.  (And even now, root can modify the
loop device using loop the device ioctls, cause a kernel oops, and
this gets reportred as a security issue.  Sigh...)  So this problem is
not unique to LLM's.  Syzbot and oss-fuzz generates a huge amount of
noise, and I've just learned to ignore it.

The bottom line is that there are differences of opinion of security
people who like to opine that we need to adhere to the most expansive
threat models --- but they don't actually fund the engineering work;
they just like to dump work on the kernel development community.  And
that opinion has probably infected Sashiko's training data.

The approach is probably to adjust Sashiko's prompts to have it
explicitly read our threat model, and to have that take precedence
over its training data.

Cheers,

						- Ted

Re: How to fix problems with the sashiko review model

[Miguel Ojeda]

On Wed, Jul 22, 2026 at 4:15 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> So you're thinking of this file:
>
> Documentation/process/threat-model.rst
>
> ?  It's already mostly in there, it's just that sashiko doesn't seem to
> be picking it up (otherwise it wouldn't have suggested the problem I
> noted), which is why I was thinking it needed to be a direct prompt ...
> unless the prompt should be to read that file?

Yeah, I had the same question for the Rust subsystem -- I have some
guidelines I want to add, which should be useful for both humans and
LLMs (e.g. not too many tokens), but I am not sure what is the best
practice here.

I think mentioning it in the prompt is needed, but perhaps it is best
to force it into the prompt `#includ`ing it somehow (ideally without
actually copy-pasting it every once in a while).

Either way, it would be nice to be consistent about this across
subsystems both on the kernel and on the prompts repository.

Cheers,
Miguel

Re: How to fix problems with the sashiko review model

[James Bottomley]

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.  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/

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

Re: How to fix problems with the sashiko review model

[Johannes Berg]

On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:
> 
> 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

I guess it's a question which way around it should be - but I'll note
that generally for wifi customers tend to not trust the "hardware"
because it's mostly firmware, is generally buggy and can be attacked
over the air too...

Personally (with that background) I'd tend to lean towards saying the
high-performance stuff that does want/need to trust the device should
opt out, it's harder to get that wrong. If we generally opt out as you
describe and then forgot to include something, we might have issues.

johannes

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 16:00 +0200, Johannes Berg wrote:
> On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:
> > 
> > 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
> 
> I guess it's a question which way around it should be - but I'll note
> that generally for wifi customers tend to not trust the "hardware"
> because it's mostly firmware, is generally buggy and can be attacked
> over the air too...
> 
> Personally (with that background) I'd tend to lean towards saying the
> high-performance stuff that does want/need to trust the device should
> opt out, it's harder to get that wrong. If we generally opt out as
> you describe and then forgot to include something, we might have
> issues.

So this is just an efficiency thing.  There are 144 driver subsystems,
so if most of the want to trust the hardware it makes more sense to
have this as default but overriden by subsystems.  However, if most of
the 144 don't trust their hardware then absolutely, I agree, it should
be per-subsystem opt in.  Part of the reason for the post was to gauge
this ... and so far I count three opt outs.

Regards,

James
 

Re: How to fix problems with the sashiko review model

[Johannes Berg]

On Wed, 2026-07-22 at 10:17 -0400, James Bottomley wrote:
> On Wed, 2026-07-22 at 16:00 +0200, Johannes Berg wrote:
> > On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:
> > > 
> > > 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
> > 
> > I guess it's a question which way around it should be - but I'll note
> > that generally for wifi customers tend to not trust the "hardware"
> > because it's mostly firmware, is generally buggy and can be attacked
> > over the air too...
> > 
> > Personally (with that background) I'd tend to lean towards saying the
> > high-performance stuff that does want/need to trust the device should
> > opt out, it's harder to get that wrong. If we generally opt out as
> > you describe and then forgot to include something, we might have
> > issues.
> 
> So this is just an efficiency thing.

Yeah I see what you're trying to get at, but I guess I just disagree.

There's also a risk thing - if you erroneously get a report for SCSI (as
you did, IIUC) then you can opt out at that point and not worry about it
again.

If we erroneously _don't_ get a report for something that didn't speak
up because they're not on the list, didn't dare say anything, didn't pay
attention, etc. then nobody ever knows until someone starts exploiting
it?

> There are 144 driver subsystems,
> so if most of the want to trust the hardware it makes more sense to
> have this as default but overriden by subsystems.  However, if most of
> the 144 don't trust their hardware then absolutely, I agree, it should
> be per-subsystem opt in.  Part of the reason for the post was to gauge
> this ... and so far I count three opt outs.

Well, you could also argue that there's one opt-out the other way around
so far (yours). You're not going to get the full set of information
required on this list either way around.

Maybe all that just means it needs to be easier to actually decide
either way. Doing it in the kernel as Greg suggested, would perhaps
lower the bar vs. some external Sashiko contribution.

johannes

Re: How to fix problems with the sashiko review model

[Steven Rostedt]

On Wed, 22 Jul 2026 16:25:03 +0200
Johannes Berg <johannes@sipsolutions.net> wrote:

> There's also a risk thing - if you erroneously get a report for SCSI (as
> you did, IIUC) then you can opt out at that point and not worry about it
> again.
> 
> If we erroneously _don't_ get a report for something that didn't speak
> up because they're not on the list, didn't dare say anything, didn't pay
> attention, etc. then nobody ever knows until someone starts exploiting
> it?

I agree with Johannes here. It's one thing to get an annoying report that
says "You can be compromised by your hardware" when the hardware is
trusted. I have the same issue with boot parameters. But I rather opt-in
than have the case that I'm the odd one out where my hardware is not
trustworthy, but I don't know I have to take action to have it checked.

That is, getting a false positive tells you that you need to take action to
opt out (perhaps the report can say "if you trust the hardware go to <link>
to opt out of this message). But it's much worse if we let security issues
escape because the maintainer didn't opt-in. As there's nothing to tell the
maintainer they need to opt-in to begin with.

-- Steve

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 11:39 -0400, Steven Rostedt wrote:
> On Wed, 22 Jul 2026 16:25:03 +0200
> Johannes Berg <johannes@sipsolutions.net> wrote:
> 
> > There's also a risk thing - if you erroneously get a report for
> > SCSI (as you did, IIUC) then you can opt out at that point and not
> > worry about it again.
> > 
> > If we erroneously _don't_ get a report for something that didn't
> > speak up because they're not on the list, didn't dare say anything,
> > didn't pay attention, etc. then nobody ever knows until someone
> > starts exploiting it?
> 
> I agree with Johannes here. It's one thing to get an annoying report
> that says "You can be compromised by your hardware" when the hardware
> is trusted. I have the same issue with boot parameters. But I rather
> opt-in than have the case that I'm the odd one out where my hardware
> is not trustworthy, but I don't know I have to take action to have it
> checked.

The thing is it's not just sashiko.  Various AI driven patchers seem to
be using the prompts, which has lead to a slew of malicious hardware
fixes being sent to linux-scsi and I'm getting a bit worn out replying
to them saying that's not in our threat model.

So I figured why not fix the problem at source.

Regards,

James

Re: How to fix problems with the sashiko review model

[Johannes Berg]

On Wed, 2026-07-22 at 11:55 -0400, James Bottomley wrote:
> On Wed, 2026-07-22 at 11:39 -0400, Steven Rostedt wrote:
> > On Wed, 22 Jul 2026 16:25:03 +0200
> > Johannes Berg <johannes@sipsolutions.net> wrote:
> > 
> > > There's also a risk thing - if you erroneously get a report for
> > > SCSI (as you did, IIUC) then you can opt out at that point and not
> > > worry about it again.
> > > 
> > > If we erroneously _don't_ get a report for something that didn't
> > > speak up because they're not on the list, didn't dare say anything,
> > > didn't pay attention, etc. then nobody ever knows until someone
> > > starts exploiting it?
> > 
> > I agree with Johannes here. It's one thing to get an annoying report
> > that says "You can be compromised by your hardware" when the hardware
> > is trusted. I have the same issue with boot parameters. But I rather
> > opt-in than have the case that I'm the odd one out where my hardware
> > is not trustworthy, but I don't know I have to take action to have it
> > checked.
> 
> The thing is it's not just sashiko.  Various AI driven patchers seem to
> be using the prompts, which has lead to a slew of malicious hardware
> fixes being sent to linux-scsi and I'm getting a bit worn out replying
> to them saying that's not in our threat model.
> 
> So I figured why not fix the problem at source.

I don't think anyone's saying you shouldn't (fix  the problem at the
source), as far as I'm concerned we're just debating the merits of opt-
in vs. opt-out :)

johannes

Re: How to fix problems with the sashiko review model

[Mauro Carvalho Chehab]

On Wed, 22 Jul 2026 10:17:40 -0400
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Wed, 2026-07-22 at 16:00 +0200, Johannes Berg wrote:
> > On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:  
> > > 
> > > 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  
> > 
> > I guess it's a question which way around it should be - but I'll note
> > that generally for wifi customers tend to not trust the "hardware"
> > because it's mostly firmware, is generally buggy and can be attacked
> > over the air too...
> > 
> > Personally (with that background) I'd tend to lean towards saying the
> > high-performance stuff that does want/need to trust the device should
> > opt out, it's harder to get that wrong. If we generally opt out as
> > you describe and then forgot to include something, we might have
> > issues.  
> 
> So this is just an efficiency thing.  There are 144 driver subsystems,
> so if most of the want to trust the hardware it makes more sense to
> have this as default but overriden by subsystems.  However, if most of
> the 144 don't trust their hardware then absolutely, I agree, it should
> be per-subsystem opt in.  Part of the reason for the post was to gauge
> this ... and so far I count three opt outs.

I don't trust hardware/firmware. They can be buggy. 

For instance, there's currently a bug at UEFI spec affecting RAS where
one particular field may have 3 different meanings because UEFI changed
its meaning on different versions of the spec without changing the version
field inside the CPER record:

	https://github.com/tianocore/edk2/issues/12708	

If even firmware specs can't be trusted, I don't think it is safe to
assume that any particular implementation of the firmware or hardware
would be safe.

Thanks,
Mauro

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 17:09 +0200, Mauro Carvalho Chehab wrote:
> On Wed, 22 Jul 2026 10:17:40 -0400
> James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> > On Wed, 2026-07-22 at 16:00 +0200, Johannes Berg wrote:
> > > On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:  
> > > > 
> > > > 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  
> > > 
> > > I guess it's a question which way around it should be - but I'll
> > > note that generally for wifi customers tend to not trust the
> > > "hardware" because it's mostly firmware, is generally buggy and
> > > can be attacked over the air too...
> > > 
> > > Personally (with that background) I'd tend to lean towards saying
> > > the high-performance stuff that does want/need to trust the
> > > device should opt out, it's harder to get that wrong. If we
> > > generally opt out as you describe and then forgot to include
> > > something, we might have issues.  
> > 
> > So this is just an efficiency thing.  There are 144 driver
> > subsystems, so if most of the want to trust the hardware it makes
> > more sense to have this as default but overriden by subsystems. 
> > However, if most of the 144 don't trust their hardware then
> > absolutely, I agree, it should be per-subsystem opt in.  Part of
> > the reason for the post was to gauge this ... and so far I count
> > three opt outs.
> 
> I don't trust hardware/firmware. They can be buggy. 

Wait, buggy is different; we have potentially buggy in SCSI as well
(rather a lot of it, in fact).  However, we don't fix theoretical
hardware bugs, we require users to specify a device (which they could
add to the code as comments to keep sashiko quiet).

The general tenor of my driver/ prompt would be that hardware may never
be assumed to be malicious but could be buggy.  In the event that the
fix is for buggy hardware, the submitter must state which hardware is
buggy and confirm they've tested the fix on the actual hardware.

Regards,

James

Re: How to fix problems with the sashiko review model

[Mauro Carvalho Chehab]

On Wed, 22 Jul 2026 11:24:38 -0400
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Wed, 2026-07-22 at 17:09 +0200, Mauro Carvalho Chehab wrote:
> > On Wed, 22 Jul 2026 10:17:40 -0400
> > James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> >   
> > > On Wed, 2026-07-22 at 16:00 +0200, Johannes Berg wrote:  
> > > > On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:    
> > > > > 
> > > > > 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    
> > > > 
> > > > I guess it's a question which way around it should be - but I'll
> > > > note that generally for wifi customers tend to not trust the
> > > > "hardware" because it's mostly firmware, is generally buggy and
> > > > can be attacked over the air too...
> > > > 
> > > > Personally (with that background) I'd tend to lean towards saying
> > > > the high-performance stuff that does want/need to trust the
> > > > device should opt out, it's harder to get that wrong. If we
> > > > generally opt out as you describe and then forgot to include
> > > > something, we might have issues.    
> > > 
> > > So this is just an efficiency thing.  There are 144 driver
> > > subsystems, so if most of the want to trust the hardware it makes
> > > more sense to have this as default but overriden by subsystems. 
> > > However, if most of the 144 don't trust their hardware then
> > > absolutely, I agree, it should be per-subsystem opt in.  Part of
> > > the reason for the post was to gauge this ... and so far I count
> > > three opt outs.  
> > 
> > I don't trust hardware/firmware. They can be buggy.   
> 
> Wait, buggy is different; we have potentially buggy in SCSI as well
> (rather a lot of it, in fact).  However, we don't fix theoretical
> hardware bugs, we require users to specify a device (which they could
> add to the code as comments to keep sashiko quiet).

That depends on the type of theoretical bug: for instance, if a value
is used to calculate an offset into an array, it makes sense to have
a check to avoid accessing data outside the array size. Also, if
a value is used as a denominator, it makes sense to check if the value
is not zero.

We do have such kind of checks on most media drivers - and there
ended helping to avoid OOPSes due to problematic hardware. That's
specially true on peripheral hardware like USB devices: they tend
to have a lot of such bugs, up to the point that we can't really
trust the device.

So, I'd say that, instead of a global prompt saying to trust hardware,
the best is to do it only on places where people are absolutely sure
that the hardware is trusty enough.

> The general tenor of my driver/ prompt would be that hardware may never
> be assumed to be malicious but could be buggy.  In the event that the
> fix is for buggy hardware, the submitter must state which hardware is
> buggy and confirm they've tested the fix on the actual hardware.

In practice, on most cases, it is hard to distinguish buggy from
malicious. IMO, it is better to prevent cases that could be 
too dangerous, even if aren't there any confirmation that the
issue happened on an actual hardware.

Thanks,
Mauro

Re: How to fix problems with the sashiko review model

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1339 bytes --]

On Wed, Jul 22, 2026 at 04:00:15PM +0200, Johannes Berg wrote:
> On Wed, 2026-07-22 at 09:42 -0400, James Bottomley wrote:

> > 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

> I guess it's a question which way around it should be - but I'll note
> that generally for wifi customers tend to not trust the "hardware"
> because it's mostly firmware, is generally buggy and can be attacked
> over the air too...

> Personally (with that background) I'd tend to lean towards saying the
> high-performance stuff that does want/need to trust the device should
> opt out, it's harder to get that wrong. If we generally opt out as you
> describe and then forgot to include something, we might have issues.

OTOH we have a huge stack of subsystems which mostly deal with devices
that are physically part of the SoC that Linux is running on or are
soldered down on the board and for the most part do not use firmware, my
guess would be that there's more code that trusts it's hardware than
doesn't.  I don't think there's any winning here.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: How to fix problems with the sashiko review model

[Guenter Roeck]

On 7/22/26 06:08, 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/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/
> 

Agree.

For Sashiko, fork and clone https://github.com/sashiko-dev/sashiko.git,
submit your changes into the clone, and send a pull request. Or at least
that is how I do it.

In the Sashiko source, per-subsystem prompts are located in directory
third_party/prompts/kernel/subsystem.

Guenter

Re: How to fix problems with the sashiko review model

[James Bottomley]

On Wed, 2026-07-22 at 06:43 -0700, Guenter Roeck wrote:
[...]
> For Sashiko, fork and clone
> https://github.com/sashiko-dev/sashiko.git,
> submit your changes into the clone, and send a pull request. Or at
> least that is how I do it.
> 
> In the Sashiko source, per-subsystem prompts are located in directory
> third_party/prompts/kernel/subsystem.

OK, now I'm confused.  I thought

https://github.com/masoncl/review-prompts

was the source of truth for this, but that project seems to have been
copied into sashiko rather than pulled in as a module.  If you want to
improve the review prompts do you now have to submit to both (which is
getting to be a bit wasteful)?

Regards,

James

Re: How to fix problems with the sashiko review model

[Miguel Ojeda]

On Wed, Jul 22, 2026 at 5:29 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> OK, now I'm confused.  I thought
>
> https://github.com/masoncl/review-prompts
>
> was the source of truth for this, but that project seems to have been
> copied into sashiko rather than pulled in as a module.  If you want to
> improve the review prompts do you now have to submit to both (which is
> getting to be a bit wasteful)?

As far as I understand Mason's is being pulled from Sashiko's, so we
submitted to Mason too.

Maybe they also sync the other way around?

Cheers,
Miguel

Re: How to fix problems with the sashiko review model

[Arnaldo Carvalho de Melo]

On Wed, Jul 22, 2026 at 11:10:45AM -0400, James Bottomley wrote:
> On Wed, 2026-07-22 at 06:43 -0700, Guenter Roeck wrote:
> [...]
> > For Sashiko, fork and clone
> > https://github.com/sashiko-dev/sashiko.git,
> > submit your changes into the clone, and send a pull request. Or at
> > least that is how I do it.

> > In the Sashiko source, per-subsystem prompts are located in directory
> > third_party/prompts/kernel/subsystem.

> OK, now I'm confused.  I thought

> https://github.com/masoncl/review-prompts

> was the source of truth for this, but that project seems to have been
> copied into sashiko rather than pulled in as a module.  If you want to
> improve the review prompts do you now have to submit to both (which is
> getting to be a bit wasteful)?

My experience is you submit to Chris's and it eventually gets pulled
into Roman's:

June 12
prompts: sync recent changes from review-prompts
https://github.com/sashiko-dev/sashiko/commit/07223d03b2657d809156b46c0b980725184f81e8

But from what I saw while searching for the above commit there are
updates to prompts in sashiko, not going first thru review-prompts.

- Arnaldo

Re: How to fix problems with the sashiko review model

[Roman Gushchin]

> On Jul 22, 2026, at 8:10 AM, James Bottomley <James.Bottomley@hansenpartnership.com> wrote:
> 
> On Wed, 2026-07-22 at 06:43 -0700, Guenter Roeck wrote:
> [...]
>> For Sashiko, fork and clone
>> https://github.com/sashiko-dev/sashiko.git,
>> submit your changes into the clone, and send a pull request. Or at
>> least that is how I do it.
>> 
>> In the Sashiko source, per-subsystem prompts are located in directory
>> third_party/prompts/kernel/subsystem.
> 
> OK, now I'm confused.  I thought
> 
> https://github.com/masoncl/review-prompts
> 
> was the source of truth for this, but that project seems to have been
> copied into sashiko rather than pulled in as a module.  If you want to
> improve the review prompts do you now have to submit to both (which is
> getting to be a bit wasteful)?

It’s a bit messy now, but Chris and me are working on something more sane.
As of now, Sashiko is using per-subsystem prompts from Chris’s repository
with some changes around more generic prompts. I periodically pull updates
from Chris’s repository. Also several folks sent prompts changes directly to
Sashiko repository, Idk if Chris is merging them back, probably not.

I also experiment with auto-generating prompts based on human feedback
to sashiko reviews, initial results are quite promising.

So as of now, I’d say send a pull request to Chris or me directly, we’ll figure out
a better model going forward.

The hard part is testing: it’s quite easy to confuse a LLM with an unlucky prompt change.
We need a stronger guidance and maybe LLM review for prompts too (sashiko for sashiko?!).
E.g. Sashiko implements a much smaller set of tools (intentionally) than a generic coding agent,
so if a prompt contains something like “run checkpatch.pl” or “search lore for …” it might be
very confusing to the LLM. When I’m pulling significant prompt updates from Chris’s repository,
I run the full benchmark to make sure we don’t get regressions.

Thanks!

Re: How to fix problems with the sashiko review model

[Roman Gushchin]

James Bottomley <James.Bottomley@HansenPartnership.com> writes:

Hi James!

> There doesn't seem to be a Sashiko list, so I added ksummit as the most
> likely one to catch attention.

We actually have one: sashiko@lists.linux.dev 

> 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.

I like the idea to inject the security threat model document from the
kernel source tree, I'll add it.

Thanks

Re: [PATCH] scsi: mpt3sas: validate variable event array spans

[James Bottomley]

On Wed, 2026-07-22 at 12:12 +0800, Pengpeng Hou wrote:
> SAS topology, PCIe topology and IR configuration events contain
> flexible arrays whose element counts are supplied by firmware. Their
> interrupt-time handlers use NumEntries or NumElements without first
> checking that the corresponding array fits in EventData.

Please explain why we might need to check this.  The device is a fat
firmware one meaning the driver is usually set up with the correctly
matching limits to the ones the firmware returns.

Regards,

James