Re: [PATCH] message: fusion: remove unused variable 'timeleft' in mptsas_issue_tm()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing Completion Reinitialization
- [High] DMA Use-After-Free / Hardware Race on Timeout
- [High] Data Race / IRQ Synchronization due to ignored timeout return
--

commit 923c21ded0be34230873d9e6304fc7cf76180fab
Author: Rosen Penev <rosenp@gmail.com>

message: fusion: remove unused variable 'timeleft' in mptsas_issue_tm()

This patch removes the timeleft variable from mptsas_issue_tm(), which was
assigned the result of wait_for_completion_timeout() but never read. The 
function instead relies on ioc->taskmgmt_cmds.status flags to determine if
the command completed successfully.

> diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
> index c362f09a8c555..82ac24bf780fc 100644
> --- a/drivers/message/fusion/mptsas.c
> +++ b/drivers/message/fusion/mptsas.c
[ ... ]
> @@ -4823,7 +4822,7 @@ mptsas_issue_tm(MPT_ADAPTER *ioc, u8 type, u8 channel, u8 id, u64 lun,
>  	mpt_put_msg_frame_hi_pri(mptsasDeviceResetCtx, ioc, mf);
>  
>  	/* Now wait for the command to complete */
> -	timeleft = wait_for_completion_timeout(&ioc->taskmgmt_cmds.done,
> +	wait_for_completion_timeout(&ioc->taskmgmt_cmds.done,
>  	    timeout*HZ);

[Severity: High]
This isn't a bug introduced by this patch, but ioc->taskmgmt_cmds.done
doesn't appear to be reinitialized via reinit_completion() before waiting.

If a previous command timed out and is later completed by a delayed
interrupt or a host reset, the completion object's internal counter would
increment. Does this cause the next call to wait_for_completion_timeout()
to return instantly by consuming the stale event, before the new command
completes?

[Severity: High]
This is a pre-existing issue, but removing the timeleft variable entrenches
a potential data race by continuing to ignore the timeout return value.

If wait_for_completion_timeout() times out, could the waking thread bypass
the complete() synchronization barrier? If mptsas_taskmgmt_complete() sets
MPT_MGMT_STATUS_COMMAND_GOOD concurrently before memcpy() finishes copying
to the reply buffer, checking the status directly without verifying the
timeout return value might cause the code to read the reply buffer while
memcpy() is actively mutating it.

>  	if (!(ioc->taskmgmt_cmds.status & MPT_MGMT_STATUS_COMMAND_GOOD)) {
>  		retval = -1; /* return failure */
>  		dtmprintk(ioc, printk(MYIOC_s_ERR_FMT
>  		    "TaskMgmt request: TIMED OUT!(mr=%p)\n", ioc->name, mf));
>  		mpt_free_msg_frame(ioc, mf);

[Severity: High]
This isn't a bug introduced by this patch, but on a timeout, the message
frame is returned to the free pool via mpt_free_msg_frame() before the
hardware is halted.

Could another thread allocate the same message frame for a new command
while the un-reset hardware still performs delayed DMA reads or writes on
it, leading to memory corruption or firmware crosstalk?

>  		if (ioc->taskmgmt_cmds.status & MPT_MGMT_STATUS_DID_IOCRESET)
>  			goto out;
>  		*issue_reset = 1;
>  		goto out;
>  	}

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