[PATCH] scsi: mptfusion: Avoid NULL alt_ioc reload in reset work

[PATCH] scsi: mptfusion: Avoid NULL alt_ioc reload in reset work

[Ruoyu Wang]

mpt_fault_reset_work() alternates delayed fault polling between bound IOC
ports by switching to ioc->alt_ioc before rearming the timer.

The peer pointer is cleared from detach and probe-error paths without
holding the task management lock used when the worker rearms itself. The
old code tested ioc->alt_ioc and then loaded it again for assignment. If
the peer clear lands between those loads, ioc becomes NULL and the
following spin_lock_irqsave(&ioc->taskmgmt_lock, flags) dereferences
NULL.

Take one READ_ONCE() snapshot and fall back to the current IOC when no
alternate is present. This preserves the existing alternating polling
behavior while removing the NULL reload window.

This issue was found by a static analysis checker and confirmed by
manual source review.

Fixes: d54d48b80fb5 ("[SCSI] mpt fusion : Adding FAULT Reset polling work")
Signed-off-by: Ruoyu Wang <ruoyuw560@gmail.com>
---
 drivers/message/fusion/mptbase.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 3a431ffd3e2eb..29bcf44356d23 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -424,8 +424,7 @@ mpt_fault_reset_work(struct work_struct *work)
 	/*
 	 * Take turns polling alternate controller
 	 */
-	if (ioc->alt_ioc)
-		ioc = ioc->alt_ioc;
+	ioc = READ_ONCE(ioc->alt_ioc) ?: ioc;
 
 	/* rearm the timer */
 	spin_lock_irqsave(&ioc->taskmgmt_lock, flags);
-- 
2.51.0

Re: [PATCH] scsi: mptfusion: Avoid NULL alt_ioc reload in reset work

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The patch resolves a TOCTOU NULL pointer dereference but leaves a severe Use-After-Free (UAF) vulnerability. If the alternate controller is detached and freed concurrently, the worker thread will dereference a freed pointer when acquiring the task management lock.
- [High] Detaching an alternate IOC permanently stops fault polling for the remaining primary IOC, silently leaving it unmonitored.
--

--- Patch [1]: [PATCH] scsi: mptfusion: Avoid NULL alt_ioc reload in reset work ---
commit 62bf3402df60ac4d2e56595bb2e871b578a2c9e5
Author: Ruoyu Wang <ruoyuw560@gmail.com>

scsi: mptfusion: Avoid NULL alt_ioc reload in reset work

mpt_fault_reset_work() alternates delayed fault polling between bound IOC
ports by switching to ioc->alt_ioc before rearming the timer.

> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 3a431ffd3e2eb..29bcf44356d23 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -424,8 +424,7 @@ mpt_fault_reset_work(struct work_struct *work)
>  	/*
>  	 * Take turns polling alternate controller
>  	 */
> -	if (ioc->alt_ioc)
> -		ioc = ioc->alt_ioc;
> +	ioc = READ_ONCE(ioc->alt_ioc) ?: ioc;

[Severity: Critical]
This is a pre-existing issue, but can this code cause a use-after-free
when the alternate controller is detached concurrently?

If the primary adapter (A) runs mpt_fault_reset_work() and reads A->alt_ioc
(pointing to B), and concurrently mpt_detach() runs for B,
mpt_adapter_dispose() clears A->alt_ioc and frees B:

drivers/message/fusion/mptbase.c:mpt_adapter_dispose() {
	if (ioc->alt_ioc)
		ioc->alt_ioc->alt_ioc = NULL;

	kfree(ioc);
}

If A's work reads A->alt_ioc just before it is cleared, will A attempt to
acquire the freed B->taskmgmt_lock below?

>  
>  	/* rearm the timer */
>  	spin_lock_irqsave(&ioc->taskmgmt_lock, flags);
>  	if (ioc->reset_work_q)
>  		queue_delayed_work(ioc->reset_work_q, &ioc->fault_reset_work,
>  			msecs_to_jiffies(MPT_POLLING_INTERVAL));

[Severity: High]
This is also a pre-existing issue, but does detaching an alternate IOC
break fault polling for the remaining primary IOC?

If the primary adapter (A) and alternate adapter (B) alternate queueing each
other's reset work, what happens when B is detached?

drivers/message/fusion/mptbase.c:mpt_detach() {
	wq = ioc->reset_work_q;
	ioc->reset_work_q = NULL;
	spin_unlock_irqrestore(&ioc->taskmgmt_lock, flags);
	cancel_delayed_work(&ioc->fault_reset_work);
}

If B's work was the currently scheduled one, it is cancelled, and A's work is
never queued again. Alternatively, if A's work evaluates the now-NULL
B->reset_work_q here, will it skip rearming the timer and leave A
without polling?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707151344.2335768-1-ruoyuw560@gmail.com?part=1