[PATCH] scsi: mptfusion: Fix potential chain array leaks on attach failure

[PATCH] scsi: mptfusion: Fix potential chain array leaks on attach failure

[Guangshuo Li]

initChainBuffers() can allocate some chain control arrays before IOC
bring-up fails. mpt_attach() then frees the adapter without releasing
those arrays.

Free the chain control arrays before freeing the adapter on this path.
This also handles partial allocation failures without invoking teardown
that requires a fully initialized adapter.

This issue was found by our static analysis tool under development.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Guangshuo Li <liguangshuo@mail.sdu.edu.cn>
---
 drivers/message/fusion/mptbase.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 3a431ffd3e2e..a0592404472c 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -2015,6 +2015,9 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
 		destroy_workqueue(ioc->reset_work_q);
 		ioc->reset_work_q = NULL;
 
+		kfree(ioc->ChainToChain);
+		kfree(ioc->RequestNB);
+		kfree(ioc->ReqToChain);
 		kfree(ioc);
 		return r;
 	}

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.43.0

Re: [PATCH] scsi: mptfusion: Fix potential chain array leaks on attach failure

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Critical] message/fusion: lockless list modification in mpt_attach()
  [1]
--

commit bd21a339f04c74107336a8df0b58bbfb54c0a200
Author: Guangshuo Li <liguangshuo@mail.sdu.edu.cn>

scsi: mptfusion: Fix potential chain array leaks on attach failure

This commit frees the ChainToChain, RequestNB, and ReqToChain arrays in
mpt_attach() when mpt_do_ioc_recovery() fails during adapter bring-up.

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 3a431ffd3e2eb..a0592404472cc 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -2015,6 +2015,9 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
>  		destroy_workqueue(ioc->reset_work_q);
>  		ioc->reset_work_q = NULL;
>  
> +		kfree(ioc->ChainToChain);
> +		kfree(ioc->RequestNB);
> +		kfree(ioc->ReqToChain);
>  		kfree(ioc);
>  		return r;
>  	}

[Severity: Critical]
This problem wasn't introduced by this patch, but can uncoordinated
modifications to the global ioc_list [1] cause a use-after-free?

mpt_attach() and mpt_adapter_dispose() add and remove ioc from ioc_list
without holding any lock.  Meanwhile, mptctl_fasync() iterates over
ioc_list under mpctl_mutex (which is private to mptctl.c and not held by
writers), and mpt_verify_adapter() iterates over ioc_list without any
lock at all.  During hotplug or sysfs bind/unbind, a reader can traverse
a broken list pointer or access a freed ioc:

CPU0                              CPU1
mpt_attach() / mpt_adapter_dispose()
                                  mptctl_fasync()
                                    mutex_lock(&mpctl_mutex)
                                    list_for_each_entry(ioc, &ioc_list, list)
  list_del(&ioc->list)
  kfree(ioc)
                                      ioc->aen_event_read_flag = 0;  <-- use-after-free

[1] https://sashiko.dev/#/bug/linux-3331f265-1868-460b-9ec0-3fe42034986b

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-4e14e563a1-scsi-mptfusion@mail.sdu.edu.cn?part=1