[PATCH] scsi: mptfusion: Fix array out of bounds error

[PATCH] scsi: mptfusion: Fix array out of bounds error

[Alexey Gladkov]

The driver retrieves the number of ports from the hardware. However, the
driver can handle no more than two such ports. It uses a fixed array for
them.

We use NumberOfPorts without checking, and maybe on actual hardware
there really are never more than two ports, but QEMU passes 8 [1][2].

[1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.h?ref_type=heads#L7
[2] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.c?ref_type=heads#L619

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
 drivers/message/fusion/mptbase.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 3a431ffd3e2e..05bd556bb938 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -3257,6 +3257,8 @@ GetPortFacts(MPT_ADAPTER *ioc, int portnum, int sleepFlag)
 		return -4;
 	}
 
+	BUG_ON(portnum < 0 || portnum >= ARRAY_SIZE(ioc->pfacts));
+
 	pfacts = &ioc->pfacts[portnum];
 
 	/* Destination (reply area)...  */
@@ -6701,6 +6703,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
 	char		 expVer[32];
 	int		 sz;
 	int		 p;
+	int		 numberOfPorts = MIN(ioc->facts.NumberOfPorts, ARRAY_SIZE(ioc->pfacts));
 
 	mpt_get_fw_exp_ver(expVer, ioc);
 
@@ -6755,7 +6758,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
 	seq_printf(m, "  MaxBuses = %d\n", ioc->facts.MaxBuses);
 
 	/* per-port info */
-	for (p=0; p < ioc->facts.NumberOfPorts; p++) {
+	for (p = 0; p < numberOfPorts; p++) {
 		seq_printf(m, "  PortNumber = %d (of %d)\n",
 				p+1,
 				ioc->facts.NumberOfPorts);
-- 
2.54.0

Re: [PATCH] scsi: mptfusion: Fix array out of bounds error

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Incomplete fix: Out-of-bounds array access in fusion sub-drivers due to unclamped ioc->facts.NumberOfPorts
--

commit c993313579593bc432c039b88b1e748a26bc690c
Author: Alexey Gladkov <legion@kernel.org>

scsi: mptfusion: Fix array out of bounds error

The driver retrieves the number of ports from the hardware. However, the
driver can handle no more than two such ports. It uses a fixed array for
them.

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 3a431ffd3e2eb..05bd556bb9383 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -3257,6 +3257,8 @@ GetPortFacts(MPT_ADAPTER *ioc, int portnum, int sleepFlag)
>  		return -4;
>  	}
>  
> +	BUG_ON(portnum < 0 || portnum >= ARRAY_SIZE(ioc->pfacts));
> +
>  	pfacts = &ioc->pfacts[portnum];

[ ... ]

> @@ -6701,6 +6703,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
>  	char		 expVer[32];
>  	int		 sz;
>  	int		 p;
> +	int		 numberOfPorts = MIN(ioc->facts.NumberOfPorts, ARRAY_SIZE(ioc->pfacts));
>  
>  	mpt_get_fw_exp_ver(expVer, ioc);
>  
> @@ -6755,7 +6758,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
>  	seq_printf(m, "  MaxBuses = %d\n", ioc->facts.MaxBuses);
>  
>  	/* per-port info */
> -	for (p=0; p < ioc->facts.NumberOfPorts; p++) {
> +	for (p = 0; p < numberOfPorts; p++) {

[Severity: Critical]
This is a pre-existing issue, but by only capping numberOfPorts locally here,
does this leave the rest of the fusion sub-drivers vulnerable to out-of-bounds
accesses?

The unclamped ioc->facts.NumberOfPorts is still used in several loops across
the driver, where the bounds of statically sized arrays (like pfacts or
fc_port_page1, both size 2) are exceeded if hardware reports more than 2
ports.

For instance, in drivers/message/fusion/mptfc.c:mptfc_remove():

	for (ii=0; ii<ioc->facts.NumberOfPorts; ii++) {
		if (ioc->fc_data.fc_port_page1[ii].data) {
			dma_free_coherent(&ioc->pcidev->dev,
					  ioc->fc_data.fc_port_page1[ii].pg_sz,
					  ioc->fc_data.fc_port_page1[ii].data,
					  ioc->fc_data.fc_port_page1[ii].dma);

Since fc_port_page1 is size 2, if NumberOfPorts is 8, could this read
adjacent struct members of MPT_ADAPTER and pass them to dma_free_coherent(),
leading to arbitrary invalid frees and a kernel panic?

Similar loops iterating over pfacts exist in mptfc_probe(), mptsas_probe(),
mptspi_probe(), and mptlan_probe().

Would it be safer to clamp ioc->facts.NumberOfPorts directly after retrieving
it from firmware, so all sub-drivers are protected?

>  		seq_printf(m, "  PortNumber = %d (of %d)\n",
>  				p+1,
>  				ioc->facts.NumberOfPorts);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616124528.319527-1-legion@kernel.org?part=1

Re: [PATCH] scsi: mptfusion: Fix array out of bounds error

[Alexey Gladkov]

On Tue, Jun 16, 2026 at 01:04:22PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [Critical] Incomplete fix: Out-of-bounds array access in fusion sub-drivers due to unclamped ioc->facts.NumberOfPorts
> --
> 
> commit c993313579593bc432c039b88b1e748a26bc690c
> Author: Alexey Gladkov <legion@kernel.org>
> 
> scsi: mptfusion: Fix array out of bounds error
> 
> The driver retrieves the number of ports from the hardware. However, the
> driver can handle no more than two such ports. It uses a fixed array for
> them.
> 
> > diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> > index 3a431ffd3e2eb..05bd556bb9383 100644
> > --- a/drivers/message/fusion/mptbase.c
> > +++ b/drivers/message/fusion/mptbase.c
> > @@ -3257,6 +3257,8 @@ GetPortFacts(MPT_ADAPTER *ioc, int portnum, int sleepFlag)
> >  		return -4;
> >  	}
> >  
> > +	BUG_ON(portnum < 0 || portnum >= ARRAY_SIZE(ioc->pfacts));
> > +
> >  	pfacts = &ioc->pfacts[portnum];
> 
> [ ... ]
> 
> > @@ -6701,6 +6703,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
> >  	char		 expVer[32];
> >  	int		 sz;
> >  	int		 p;
> > +	int		 numberOfPorts = MIN(ioc->facts.NumberOfPorts, ARRAY_SIZE(ioc->pfacts));
> >  
> >  	mpt_get_fw_exp_ver(expVer, ioc);
> >  
> > @@ -6755,7 +6758,7 @@ static int mpt_iocinfo_proc_show(struct seq_file *m, void *v)
> >  	seq_printf(m, "  MaxBuses = %d\n", ioc->facts.MaxBuses);
> >  
> >  	/* per-port info */
> > -	for (p=0; p < ioc->facts.NumberOfPorts; p++) {
> > +	for (p = 0; p < numberOfPorts; p++) {
> 
> [Severity: Critical]
> This is a pre-existing issue, but by only capping numberOfPorts locally here,
> does this leave the rest of the fusion sub-drivers vulnerable to out-of-bounds
> accesses?
> 
> The unclamped ioc->facts.NumberOfPorts is still used in several loops across
> the driver, where the bounds of statically sized arrays (like pfacts or
> fc_port_page1, both size 2) are exceeded if hardware reports more than 2
> ports.
> 
> For instance, in drivers/message/fusion/mptfc.c:mptfc_remove():
> 
> 	for (ii=0; ii<ioc->facts.NumberOfPorts; ii++) {
> 		if (ioc->fc_data.fc_port_page1[ii].data) {
> 			dma_free_coherent(&ioc->pcidev->dev,
> 					  ioc->fc_data.fc_port_page1[ii].pg_sz,
> 					  ioc->fc_data.fc_port_page1[ii].data,
> 					  ioc->fc_data.fc_port_page1[ii].dma);
> 
> Since fc_port_page1 is size 2, if NumberOfPorts is 8, could this read
> adjacent struct members of MPT_ADAPTER and pass them to dma_free_coherent(),
> leading to arbitrary invalid frees and a kernel panic?
> 
> Similar loops iterating over pfacts exist in mptfc_probe(), mptsas_probe(),
> mptspi_probe(), and mptlan_probe().
> 
> Would it be safer to clamp ioc->facts.NumberOfPorts directly after retrieving
> it from firmware, so all sub-drivers are protected?

Indeed. We could create a wrapper to retrieve MPT_ADAPTER that checks for
array index out-of-bounds errors. But there is one little catch here,
since we use NumberOfPorts to iterate over different arrays. We need to
synchronize their sizes.

> >  		seq_printf(m, "  PortNumber = %d (of %d)\n",
> >  				p+1,
> >  				ioc->facts.NumberOfPorts);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260616124528.319527-1-legion@kernel.org?part=1
> 

-- 
Rgrds, legion

[PATCH v2] scsi: mptfusion: Fix array out of bounds error

[Alexey Gladkov]

The driver retrieves the number of ports from the hardware. However, the
driver can handle no more than two such ports. It uses a fixed array for
them.

We use NumberOfPorts without checking, and maybe on actual hardware
there really are never more than two ports, but QEMU passes 8 [1][2].

[1] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.h?ref_type=heads#L7
[2] https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.c?ref_type=heads#L619

Signed-off-by: Alexey Gladkov <legion@kernel.org>
---
v2:
* Replaced the retrieval of MPT_ADAPTER from pci_get_drvdata with a wrapper that
  checks the array boundaries.
* Replaced the magic number of array elements with a macro because these arrays
  are associated.

---
 drivers/message/fusion/mptbase.c  | 22 +++++++++++++++++++---
 drivers/message/fusion/mptbase.h  |  9 ++++++---
 drivers/message/fusion/mptctl.c   |  2 +-
 drivers/message/fusion/mptfc.c    |  4 ++--
 drivers/message/fusion/mptlan.c   |  4 ++--
 drivers/message/fusion/mptsas.c   |  6 +++---
 drivers/message/fusion/mptscsih.c |  6 +++---
 drivers/message/fusion/mptspi.c   |  6 +++---
 8 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 3a431ffd3e2e..9e738d0bb8e3 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -1741,6 +1741,21 @@ mpt_mapresources(MPT_ADAPTER *ioc)
 	return r;
 }
 
+MPT_ADAPTER *
+mpt_get_adapter(struct pci_dev *pdev)
+{
+	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+
+	if (ioc && ioc->facts.NumberOfPorts >= ARRAY_SIZE(ioc->pfacts)) {
+		ioc->facts.NumberOfPorts = ARRAY_SIZE(ioc->pfacts);
+	}
+
+	BUILD_BUG_ON(ARRAY_SIZE(ioc->pfacts) != ARRAY_SIZE(ioc->fc_port_page0));
+	BUILD_BUG_ON(ARRAY_SIZE(ioc->pfacts) != ARRAY_SIZE(ioc->fc_data.fc_port_page1));
+
+	return ioc;
+}
+
 /*=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=*/
 /**
  *	mpt_attach - Install a PCI intelligent MPT adapter.
@@ -2074,7 +2089,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 void
 mpt_detach(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 	*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 	*ioc = mpt_get_adapter(pdev);
 	char pname[64];
 	u8 cb_idx;
 	unsigned long flags;
@@ -2140,7 +2155,7 @@ int
 mpt_suspend(struct pci_dev *pdev, pm_message_t state)
 {
 	u32 device_state;
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 
 	device_state = pci_choose_state(pdev, state);
 	printk(MYIOC_s_INFO_FMT "pci-suspend: pdev=0x%p, slot=%s, Entering "
@@ -2179,7 +2194,7 @@ mpt_suspend(struct pci_dev *pdev, pm_message_t state)
 int
 mpt_resume(struct pci_dev *pdev)
 {
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 	u32 device_state = pdev->current_state;
 	int recovery_state;
 	int err;
@@ -8451,6 +8466,7 @@ EXPORT_SYMBOL(mpt_reset_register);
 EXPORT_SYMBOL(mpt_reset_deregister);
 EXPORT_SYMBOL(mpt_device_driver_register);
 EXPORT_SYMBOL(mpt_device_driver_deregister);
+EXPORT_SYMBOL(mpt_get_adapter);
 EXPORT_SYMBOL(mpt_get_msg_frame);
 EXPORT_SYMBOL(mpt_put_msg_frame);
 EXPORT_SYMBOL(mpt_put_msg_frame_hi_pri);
diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
index b406fd676da0..ca0a873af01e 100644
--- a/drivers/message/fusion/mptbase.h
+++ b/drivers/message/fusion/mptbase.h
@@ -113,6 +113,8 @@
 #define MPT_PROCFS_SUMMARY_ALL_PATHNAME		"/proc/" MPT_PROCFS_SUMMARY_ALL_NODE
 #define MPT_FW_REV_MAGIC_ID_STRING		"FwRev="
 
+#define MPT_MAX_PORT_FACTS		2
+
 #define  MPT_MAX_REQ_DEPTH		1023
 #define  MPT_DEFAULT_REQ_DEPTH		256
 #define  MPT_MIN_REQ_DEPTH		128
@@ -537,7 +539,7 @@ typedef struct _FcCfgData {
 		FCPortPage1_t	*data;
 		dma_addr_t	 dma;
 		int		 pg_sz;
-	}			 fc_port_page1[2];
+	}			 fc_port_page1[MPT_MAX_PORT_FACTS];
 } FcCfgData;
 
 #define MPT_RPORT_INFO_FLAGS_REGISTERED	0x01	/* rport registered */
@@ -699,8 +701,8 @@ typedef struct _MPT_ADAPTER
 	u32			 hs_req[MPT_MAX_FRAME_SIZE/sizeof(u32)];
 	u16			 hs_reply[MPT_MAX_FRAME_SIZE/sizeof(u16)];
 	IOCFactsReply_t		 facts;
-	PortFactsReply_t	 pfacts[2];
-	FCPortPage0_t		 fc_port_page0[2];
+	PortFactsReply_t	 pfacts[MPT_MAX_PORT_FACTS];
+	FCPortPage0_t		 fc_port_page0[MPT_MAX_PORT_FACTS];
 	LANPage0_t		 lan_cnfg_page0;
 	LANPage1_t		 lan_cnfg_page1;
 
@@ -918,6 +920,7 @@ extern int	 mpt_reset_register(u8 cb_idx, MPT_RESETHANDLER reset_func);
 extern void	 mpt_reset_deregister(u8 cb_idx);
 extern int	 mpt_device_driver_register(struct mpt_pci_driver * dd_cbfunc, u8 cb_idx);
 extern void	 mpt_device_driver_deregister(u8 cb_idx);
+extern MPT_ADAPTER *mpt_get_adapter(struct pci_dev *pdev);
 extern MPT_FRAME_HDR	*mpt_get_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc);
 extern void	 mpt_free_msg_frame(MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
 extern void	 mpt_put_msg_frame(u8 cb_idx, MPT_ADAPTER *ioc, MPT_FRAME_HDR *mf);
diff --git a/drivers/message/fusion/mptctl.c b/drivers/message/fusion/mptctl.c
index 77fa55df70d0..8023062431e1 100644
--- a/drivers/message/fusion/mptctl.c
+++ b/drivers/message/fusion/mptctl.c
@@ -2849,7 +2849,7 @@ static long compat_mpctl_ioctl(struct file *f, unsigned int cmd, unsigned long a
 static int
 mptctl_probe(struct pci_dev *pdev)
 {
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 
 	mutex_init(&ioc->ioctl_cmds.mutex);
 	init_completion(&ioc->ioctl_cmds.done);
diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
index b55deb988ad9..3c00f4c88343 100644
--- a/drivers/message/fusion/mptfc.c
+++ b/drivers/message/fusion/mptfc.c
@@ -1219,7 +1219,7 @@ mptfc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if ((r = mpt_attach(pdev,id)) != 0)
 		return r;
 
-	ioc = pci_get_drvdata(pdev);
+	ioc = mpt_get_adapter(pdev);
 	ioc->DoneCtx = mptfcDoneCtx;
 	ioc->TaskCtx = mptfcTaskCtx;
 	ioc->InternalCtx = mptfcInternalCtx;
@@ -1525,7 +1525,7 @@ mptfc_init(void)
  */
 static void mptfc_remove(struct pci_dev *pdev)
 {
-	MPT_ADAPTER		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER		*ioc = mpt_get_adapter(pdev);
 	struct mptfc_rport_info	*p, *n;
 	struct workqueue_struct *work_q;
 	unsigned long		flags;
diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index f7fc5cc04b92..ccd735aa532a 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -1380,7 +1380,7 @@ mpt_register_lan_device (MPT_ADAPTER *mpt_dev, int pnum)
 static int
 mptlan_probe(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 		*ioc = mpt_get_adapter(pdev);
 	struct net_device	*dev;
 	int			i;
 
@@ -1426,7 +1426,7 @@ mptlan_probe(struct pci_dev *pdev)
 static void
 mptlan_remove(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 		*ioc = mpt_get_adapter(pdev);
 	struct net_device	*dev = ioc->netdev;
 	struct mpt_lan_priv *priv = netdev_priv(dev);
 
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index c362f09a8c55..b328c8685192 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -5163,7 +5163,7 @@ mptsas_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (r)
 		return r;
 
-	ioc = pci_get_drvdata(pdev);
+	ioc = mpt_get_adapter(pdev);
 	mptsas_fw_event_off(ioc);
 	ioc->DoneCtx = mptsasDoneCtx;
 	ioc->TaskCtx = mptsasTaskCtx;
@@ -5337,7 +5337,7 @@ mptsas_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 static void
 mptsas_shutdown(struct pci_dev *pdev)
 {
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 
 	mptsas_fw_event_off(ioc);
 	mptsas_cleanup_fw_event_q(ioc);
@@ -5345,7 +5345,7 @@ mptsas_shutdown(struct pci_dev *pdev)
 
 static void mptsas_remove(struct pci_dev *pdev)
 {
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 	struct mptsas_portinfo *p, *n;
 	int i;
 
diff --git a/drivers/message/fusion/mptscsih.c b/drivers/message/fusion/mptscsih.c
index ec6edcc4ef56..290939225688 100644
--- a/drivers/message/fusion/mptscsih.c
+++ b/drivers/message/fusion/mptscsih.c
@@ -1171,7 +1171,7 @@ mptscsih_report_queue_full(struct scsi_cmnd *sc, SCSIIOReply_t *pScsiReply, SCSI
 void
 mptscsih_remove(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 		*ioc = mpt_get_adapter(pdev);
 	struct Scsi_Host 	*host = ioc->sh;
 	MPT_SCSI_HOST		*hd;
 	int sz1;
@@ -1228,7 +1228,7 @@ mptscsih_shutdown(struct pci_dev *pdev)
 int
 mptscsih_suspend(struct pci_dev *pdev, pm_message_t state)
 {
-	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 		*ioc = mpt_get_adapter(pdev);
 
 	scsi_block_requests(ioc->sh);
 	mptscsih_shutdown(pdev);
@@ -1244,7 +1244,7 @@ mptscsih_suspend(struct pci_dev *pdev, pm_message_t state)
 int
 mptscsih_resume(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 		*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 		*ioc = mpt_get_adapter(pdev);
 	int rc;
 
 	rc = mpt_resume(pdev);
diff --git a/drivers/message/fusion/mptspi.c b/drivers/message/fusion/mptspi.c
index 56892b1f3de2..ad92c23bd632 100644
--- a/drivers/message/fusion/mptspi.c
+++ b/drivers/message/fusion/mptspi.c
@@ -1330,7 +1330,7 @@ mptspi_ioc_reset(MPT_ADAPTER *ioc, int reset_phase)
 static int
 mptspi_resume(struct pci_dev *pdev)
 {
-	MPT_ADAPTER 	*ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER 	*ioc = mpt_get_adapter(pdev);
 	struct _MPT_SCSI_HOST *hd = shost_priv(ioc->sh);
 	int rc;
 
@@ -1367,7 +1367,7 @@ mptspi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if ((r = mpt_attach(pdev,id)) != 0)
 		return r;
 
-	ioc = pci_get_drvdata(pdev);
+	ioc = mpt_get_adapter(pdev);
 	ioc->DoneCtx = mptspiDoneCtx;
 	ioc->TaskCtx = mptspiTaskCtx;
 	ioc->InternalCtx = mptspiInternalCtx;
@@ -1546,7 +1546,7 @@ mptspi_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 static void mptspi_remove(struct pci_dev *pdev)
 {
-	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
+	MPT_ADAPTER *ioc = mpt_get_adapter(pdev);
 
 	scsi_remove_host(ioc->sh);
 	mptscsih_remove(pdev);
-- 
2.54.0

Re: [PATCH v2] scsi: mptfusion: Fix array out of bounds error

[sashiko-bot]

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

Pre-existing issues:
- [High] The patch attempts to fix an array out-of-bounds error by clamping `ioc->facts.NumberOfPorts` to `ARRAY_SIZE(ioc->pfacts)` inside a newly introduced `mpt_get_adapter()` wrapper. However, this fix is incomplete and easily bypassed. The underlying hardware facts are dynamically refreshed during background hardware fault recovery by `GetIocFacts()`, which directly overwrites `ioc->facts` with raw firmware data using `memset` and hardware replies. Because `mpt_get_adapter()` is only called during device probing and suspend/resume, background recovery resets `NumberOfPorts` to the unbounded hardware value (e.g., 8). Subsequently, unprivileged users can read the S_IRUGO `/proc/mpt/iocX/info` file, triggering `mpt_iocinfo_proc_show()`. This function loops over the unbounded `NumberOfPorts`, causing out-of-bounds reads on the `pfacts` and `fc_port_page0` arrays (both sized to 2), leaking adjacent `MPT_ADAPTER` memory to user-space.
--

commit 09bc8d96f319385694c9b6744f49e39d39f37f63
Author: Alexey Gladkov <legion@kernel.org>

scsi: mptfusion: Fix array out of bounds error

The driver retrieves the number of ports from the hardware. However, the
driver can handle no more than two such ports and uses a fixed array.
This patch introduces mpt_get_adapter() to clamp the number of ports
to the array size to prevent out-of-bounds errors.

Link: https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.h?ref_type=heads#L7
Link: https://gitlab.com/qemu-project/qemu/-/blob/master/hw/scsi/mptsas.c?ref_type=heads#L619

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 3a431ffd3e2eb..9e738d0bb8e35 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c

[ ... ]

> @@ -1741,6 +1741,21 @@ mpt_mapresources(MPT_ADAPTER *ioc)
>  	return r;
>  }
>  
> +MPT_ADAPTER *
> +mpt_get_adapter(struct pci_dev *pdev)
> +{
> +	MPT_ADAPTER *ioc = pci_get_drvdata(pdev);
> +
> +	if (ioc && ioc->facts.NumberOfPorts >= ARRAY_SIZE(ioc->pfacts)) {
> +		ioc->facts.NumberOfPorts = ARRAY_SIZE(ioc->pfacts);
> +	}
> +

[Severity: High]
This is a pre-existing issue, but does this fix completely prevent the
out-of-bounds access?

While this wrapper clamps the value during device probing and suspend/resume,
background recovery operations bypass it. Specifically, GetIocFacts() writes
raw firmware data directly into ioc->facts:

GetIocFacts() {
    facts = &ioc->facts;
    reply_sz = sizeof(*facts);
    memset(facts, 0, reply_sz);
    ... 
    r = mpt_handshake_req_reply_wait(ioc, req_sz, (u32*)&get_facts,
                                     reply_sz, (u16*)facts, ...);
}

If the hardware (or QEMU) provides a NumberOfPorts greater than 2 during
background recovery, the clamp is lost. Later, an unprivileged user reading
the S_IRUGO file /proc/mpt/iocX/info triggers mpt_iocinfo_proc_show(), which
uses the unclamped value:

mpt_iocinfo_proc_show() {
    for (p=0; p < ioc->facts.NumberOfPorts; p++) {
        ...
        ioc->fc_port_page0[p].WWNN.High,
}

Could this still lead to an out-of-bounds array read that leaks kernel
memory to user-space? Would it be safer to enforce this clamp directly
inside GetIocFacts() where the hardware data is received?

> +	BUILD_BUG_ON(ARRAY_SIZE(ioc->pfacts) != ARRAY_SIZE(ioc->fc_port_page0));
> +	BUILD_BUG_ON(ARRAY_SIZE(ioc->pfacts) != ARRAY_SIZE(ioc->fc_data.fc_port_page1));
> +
> +	return ioc;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616152908.363621-1-legion@kernel.org?part=1