[PATCH] scsi: pmcraid: shut down command timers before reuse

[PATCH] scsi: pmcraid: shut down command timers before reuse

[Runyu Xiao]

pmcraid removes a command from the pending pool and then either
completes it or reuses it for reset. timer_delete() only removes a
pending timer; it does not wait for a callback already running and does
not prevent the callback from rearming the timer. A timeout callback can
therefore access a command block after it has been returned or
reinitialized.

The response paths run in hardirq or softirq context, so they cannot
wait synchronously for a normal timer callback. Shut down the timer there
and defer completion to a work item. The worker uses
timer_shutdown_sync() before calling the original completion function.
The reset cleanup path also defers the reset command when it is still
pending so the reset engine cannot reinitialize it before its timer
callback has finished.

Flush command work before releasing command and control buffers.

Fixes: 89a3681041507773 ("[SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller")

Cc: stable@vger.kernel.org

Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index 942a99393204..ed2ebe68c816 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -55,6 +55,43 @@ static unsigned int pmcraid_enable_msix;
  */
 static atomic_t pmcraid_adapter_count = ATOMIC_INIT(0);
 
+static void pmcraid_cmd_work(struct work_struct *work);
+static void pmcraid_complete_reset_cmd(struct pmcraid_cmd *cmd);
+
+static void pmcraid_complete_reset_cmd(struct pmcraid_cmd *cmd)
+{
+	struct pmcraid_instance *pinstance = cmd->drv_inst;
+	unsigned long lock_flags;
+
+	spin_lock_irqsave(pinstance->host->host_lock, lock_flags);
+	cmd->cmd_done(cmd);
+	spin_unlock_irqrestore(pinstance->host->host_lock, lock_flags);
+}
+
+static void pmcraid_complete_response_cmd(struct pmcraid_cmd *cmd)
+{
+	cmd->cmd_done(cmd);
+}
+
+static void pmcraid_schedule_cmd_work(struct pmcraid_cmd *cmd,
+				      void (*work_fn)(struct pmcraid_cmd *))
+{
+	/* The command block stays unavailable until the worker completes. */
+	timer_shutdown(&cmd->timer);
+	cmd->work_fn = work_fn;
+	schedule_work(&cmd->timer_work);
+}
+
+static void pmcraid_cmd_work(struct work_struct *work)
+{
+	struct pmcraid_cmd *cmd = container_of(work, struct pmcraid_cmd,
+			timer_work);
+	void (*work_fn)(struct pmcraid_cmd *) = cmd->work_fn;
+
+	timer_shutdown_sync(&cmd->timer);
+	work_fn(cmd);
+}
+
 /*
  * Supporting user-level control interface through IOCTL commands.
  * pmcraid_major - major number to use
@@ -330,6 +367,7 @@ static void pmcraid_init_cmdblk(struct pmcraid_cmd *cmd, int index)
 	}
 
 	cmd->cmd_done = NULL;
+	cmd->work_fn = NULL;
 	cmd->scsi_cmd = NULL;
 	cmd->release = 0;
 	cmd->completion_req = 0;
@@ -483,8 +521,6 @@ static void pmcraid_clr_trans_op(
 	struct pmcraid_instance *pinstance
 )
 {
-	unsigned long lock_flags;
-
 	if (!pinstance->interrupt_mode) {
 		iowrite32(INTRS_TRANSITION_TO_OPERATIONAL,
 			pinstance->int_regs.ioa_host_interrupt_mask_reg);
@@ -495,12 +531,8 @@ static void pmcraid_clr_trans_op(
 	}
 
 	if (pinstance->reset_cmd != NULL) {
-		timer_delete(&pinstance->reset_cmd->timer);
-		spin_lock_irqsave(
-			pinstance->host->host_lock, lock_flags);
-		pinstance->reset_cmd->cmd_done(pinstance->reset_cmd);
-		spin_unlock_irqrestore(
-			pinstance->host->host_lock, lock_flags);
+		pmcraid_schedule_cmd_work(pinstance->reset_cmd,
+					  pmcraid_complete_reset_cmd);
 	}
 }
 
@@ -1967,6 +1999,32 @@ static void pmcraid_get_dump(struct pmcraid_instance *pinstance)
 	pmcraid_info("%s is not yet implemented\n", __func__);
 }
 
+static void pmcraid_fail_cmd(struct pmcraid_cmd *cmd)
+{
+	struct pmcraid_instance *pinstance = cmd->drv_inst;
+	unsigned long lock_flags;
+
+	spin_lock_irqsave(pinstance->host->host_lock, lock_flags);
+	if (cmd->scsi_cmd) {
+		struct scsi_cmnd *scsi_cmd = cmd->scsi_cmd;
+		__le32 resp = cmd->ioa_cb->ioarcb.response_handle;
+		u8 cdb = cmd->ioa_cb->ioarcb.cdb[0];
+
+		scsi_cmd->result |= DID_ERROR << 16;
+		scsi_dma_unmap(scsi_cmd);
+		pmcraid_info("failing(%d) CDB[0] = %x result: %x\n",
+			     le32_to_cpu(resp) >> 2, cdb, scsi_cmd->result);
+		pmcraid_return_cmd(cmd);
+		scsi_done(scsi_cmd);
+	} else if (cmd->cmd_done == pmcraid_internal_done ||
+		   cmd->cmd_done == pmcraid_erp_done) {
+		cmd->cmd_done(cmd);
+	} else {
+		pmcraid_return_cmd(cmd);
+	}
+	spin_unlock_irqrestore(pinstance->host->host_lock, lock_flags);
+}
+
 /**
  * pmcraid_fail_outstanding_cmds - Fails all outstanding ops.
  * @pinstance: pointer to adapter instance structure
@@ -1977,12 +2035,13 @@ static void pmcraid_get_dump(struct pmcraid_instance *pinstance)
  * pool.
  *
  * Return value:
- *	 none
+ *	 true if reset command completion was deferred, otherwise false
  */
-static void pmcraid_fail_outstanding_cmds(struct pmcraid_instance *pinstance)
+static bool pmcraid_fail_outstanding_cmds(struct pmcraid_instance *pinstance)
 {
 	struct pmcraid_cmd *cmd, *temp;
 	unsigned long lock_flags;
+	bool reset_deferred = false;
 
 	/* pending command list is protected by pending_pool_lock. Its
 	 * traversal must be done as within this lock
@@ -1998,42 +2057,28 @@ static void pmcraid_fail_outstanding_cmds(struct pmcraid_instance *pinstance)
 		cmd->ioa_cb->ioasa.ilid =
 			cpu_to_le32(PMCRAID_DRIVER_ILID);
 
-		/* In case the command timer is still running */
-		timer_delete(&cmd->timer);
-
-		/* If this is an IO command, complete it by invoking scsi_done
-		 * function. If this is one of the internal commands other
-		 * than pmcraid_ioa_reset and HCAM commands invoke cmd_done to
-		 * complete it
-		 */
-		if (cmd->scsi_cmd) {
-
-			struct scsi_cmnd *scsi_cmd = cmd->scsi_cmd;
-			__le32 resp = cmd->ioa_cb->ioarcb.response_handle;
-
-			scsi_cmd->result |= DID_ERROR << 16;
-
-			scsi_dma_unmap(scsi_cmd);
-			pmcraid_return_cmd(cmd);
-
-			pmcraid_info("failing(%d) CDB[0] = %x result: %x\n",
-				     le32_to_cpu(resp) >> 2,
-				     cmd->ioa_cb->ioarcb.cdb[0],
-				     scsi_cmd->result);
-			scsi_done(scsi_cmd);
-		} else if (cmd->cmd_done == pmcraid_internal_done ||
-			   cmd->cmd_done == pmcraid_erp_done) {
-			cmd->cmd_done(cmd);
-		} else if (cmd->cmd_done != pmcraid_ioa_reset &&
-			   cmd->cmd_done != pmcraid_ioa_shutdown_done) {
-			pmcraid_return_cmd(cmd);
-		}
-
 		atomic_dec(&pinstance->outstanding_cmds);
+		if (cmd == pinstance->reset_cmd &&
+		    cmd->cmd_done == pmcraid_ioa_reset) {
+			/* The reset engine owns this command and must resume only
+			 * after its timer callback has finished.
+			 */
+			pmcraid_schedule_cmd_work(cmd, pmcraid_complete_reset_cmd);
+			reset_deferred = true;
+		} else if (cmd == pinstance->reset_cmd &&
+			   cmd->cmd_done == pmcraid_ioa_shutdown_done) {
+			/* pmcraid_ioa_shutdown_done() takes host_lock itself. */
+			pmcraid_schedule_cmd_work(cmd,
+						  pmcraid_complete_response_cmd);
+			reset_deferred = true;
+		} else {
+			pmcraid_schedule_cmd_work(cmd, pmcraid_fail_cmd);
+		}
 		spin_lock_irqsave(&pinstance->pending_pool_lock, lock_flags);
 	}
 
 	spin_unlock_irqrestore(&pinstance->pending_pool_lock, lock_flags);
+	return reset_deferred;
 }
 
 /**
@@ -2151,8 +2196,11 @@ static void pmcraid_ioa_reset(struct pmcraid_cmd *cmd)
 		 */
 		pci_restore_state(pinstance->pdev);
 
-		/* fail all pending commands */
-		pmcraid_fail_outstanding_cmds(pinstance);
+		/* fail all pending commands. If the reset command itself is still
+		 * pending, its timer must finish before the reset engine reuses it.
+		 */
+		if (pmcraid_fail_outstanding_cmds(pinstance))
+			break;
 
 		/* check if unit check is active, if so extract dump */
 		if (pinstance->ioa_unit_check) {
@@ -3934,7 +3982,6 @@ static void pmcraid_tasklet_function(unsigned long instance)
 	struct pmcraid_instance *pinstance;
 	unsigned long hrrq_lock_flags;
 	unsigned long pending_lock_flags;
-	unsigned long host_lock_flags;
 	spinlock_t *lockp; /* hrrq buffer lock */
 	int id;
 	u32 resp;
@@ -3982,17 +4029,15 @@ static void pmcraid_tasklet_function(unsigned long instance)
 		list_del(&cmd->free_list);
 		spin_unlock_irqrestore(&pinstance->pending_pool_lock,
 					pending_lock_flags);
-		timer_delete(&cmd->timer);
 		atomic_dec(&pinstance->outstanding_cmds);
 
 		if (cmd->cmd_done == pmcraid_ioa_reset) {
-			spin_lock_irqsave(pinstance->host->host_lock,
-					  host_lock_flags);
-			cmd->cmd_done(cmd);
-			spin_unlock_irqrestore(pinstance->host->host_lock,
-					       host_lock_flags);
+			pmcraid_schedule_cmd_work(cmd, pmcraid_complete_reset_cmd);
 		} else if (cmd->cmd_done != NULL) {
-			cmd->cmd_done(cmd);
+			pmcraid_schedule_cmd_work(cmd,
+						  pmcraid_complete_response_cmd);
+		} else {
+			timer_shutdown(&cmd->timer);
 		}
 		/* loop over until we are done with all responses */
 		spin_lock_irqsave(lockp, hrrq_lock_flags);
@@ -4097,7 +4142,10 @@ pmcraid_release_cmd_blocks(struct pmcraid_instance *pinstance, int max_index)
 {
 	int i;
 	for (i = 0; i < max_index; i++) {
-		kmem_cache_free(pinstance->cmd_cachep, pinstance->cmd_list[i]);
+		struct pmcraid_cmd *cmd = pinstance->cmd_list[i];
+
+		flush_work(&cmd->timer_work);
+		kmem_cache_free(pinstance->cmd_cachep, cmd);
 		pinstance->cmd_list[i] = NULL;
 	}
 	kmem_cache_destroy(pinstance->cmd_cachep);
@@ -4137,6 +4185,14 @@ pmcraid_release_control_blocks(
 	pinstance->control_pool = NULL;
 }
 
+static void pmcraid_flush_cmd_works(struct pmcraid_instance *pinstance)
+{
+	int i;
+
+	for (i = 0; i < PMCRAID_MAX_CMD; i++)
+		flush_work(&pinstance->cmd_list[i]->timer_work);
+}
+
 /**
  * pmcraid_allocate_cmd_blocks - allocate memory for cmd block structures
  * @pinstance: pointer to per adapter instance structure
@@ -4168,6 +4224,7 @@ static int pmcraid_allocate_cmd_blocks(struct pmcraid_instance *pinstance)
 			pmcraid_release_cmd_blocks(pinstance, i);
 			return -ENOMEM;
 		}
+		INIT_WORK(&pinstance->cmd_list[i]->timer_work, pmcraid_cmd_work);
 	}
 	return 0;
 }
@@ -4459,6 +4516,7 @@ static void pmcraid_kill_tasklets(struct pmcraid_instance *pinstance)
  */
 static void pmcraid_release_buffers(struct pmcraid_instance *pinstance)
 {
+	pmcraid_flush_cmd_works(pinstance);
 	pmcraid_release_config_buffers(pinstance);
 	pmcraid_release_control_blocks(pinstance, PMCRAID_MAX_CMD);
 	pmcraid_release_cmd_blocks(pinstance, PMCRAID_MAX_CMD);
diff --git a/drivers/scsi/pmcraid.h b/drivers/scsi/pmcraid.h
index cd059b7599b4..dd4f18d14586 100644
--- a/drivers/scsi/pmcraid.h
+++ b/drivers/scsi/pmcraid.h
@@ -14,6 +14,7 @@
 #include <linux/types.h>
 #include <linux/completion.h>
 #include <linux/list.h>
+#include <linux/workqueue.h>
 #include <scsi/scsi.h>
 #include <scsi/scsi_cmnd.h>
 #include <linux/cdev.h>
@@ -569,12 +570,14 @@ struct pmcraid_cmd {
 	struct list_head free_list;
 	struct completion wait_for_completion;
 	struct timer_list timer;	/* needed for internal commands */
+	struct work_struct timer_work;
 	u32 timeout;			/* current timeout value */
 	u32 index;			/* index into the command list */
 	u8 completion_req;		/* for handling internal commands */
 	u8 release;			/* for handling completions */
 
 	void (*cmd_done) (struct pmcraid_cmd *);
+	void (*work_fn)(struct pmcraid_cmd *cmd);
 	struct pmcraid_instance *drv_inst;
 
 	struct pmcraid_sglist *sglist; /* used for passthrough IOCTLs */
-- 
2.34.1

Re: [PATCH] scsi: pmcraid: shut down command timers before reuse

[Greg KH]

On Fri, Aug 21, 2026 at 02:23:51PM +0800, Runyu Xiao wrote:
> pmcraid removes a command from the pending pool and then either
> completes it or reuses it for reset. timer_delete() only removes a
> pending timer; it does not wait for a callback already running and does
> not prevent the callback from rearming the timer. A timeout callback can
> therefore access a command block after it has been returned or
> reinitialized.
> 
> The response paths run in hardirq or softirq context, so they cannot
> wait synchronously for a normal timer callback. Shut down the timer there
> and defer completion to a work item. The worker uses
> timer_shutdown_sync() before calling the original completion function.
> The reset cleanup path also defers the reset command when it is still
> pending so the reset engine cannot reinitialize it before its timer
> callback has finished.
> 
> Flush command work before releasing command and control buffers.

How was this found and tested?

> 
> Fixes: 89a3681041507773 ("[SCSI] pmcraid: PMC-Sierra MaxRAID driver to support 6Gb/s SAS RAID controller")
> 
> Cc: stable@vger.kernel.org
> 
> Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>

Why the extra whitespace?  Didn't checkpatch complain about this?


> 
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index 942a99393204..ed2ebe68c816 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -55,6 +55,43 @@ static unsigned int pmcraid_enable_msix;
>   */
>  static atomic_t pmcraid_adapter_count = ATOMIC_INIT(0);
>  
> +static void pmcraid_cmd_work(struct work_struct *work);
> +static void pmcraid_complete_reset_cmd(struct pmcraid_cmd *cmd);
> +
> +static void pmcraid_complete_reset_cmd(struct pmcraid_cmd *cmd)

Why the prototype right before the definition?  Did a LLM create this
patch?  If so, you should upgrade to a better coding model :)

thanks,

greg k-h

Re: [PATCH] scsi: pmcraid: shut down command timers before reuse

[Runyu Xiao]

On Fri, Aug 21, 2026 at 02:23:51PM +0800, Greg Kroah-Hartman wrote:
> How was this found and tested?

The issue was found by a static-analysis check and then confirmed by
tracing the command ownership and timer lifetime in pmcraid.c. The v1
patch was generated by an LLM from that finding.

I did not reproduce this with pmcraid hardware. I only ran
git apply --check, checkpatch.pl --strict, and built
drivers/scsi/pmcraid.o against current mainline. I should not have
presented the result as sufficiently tested.

> Why the extra whitespace? Didn't checkpatch complain about this?

I rechecked the exact patch. checkpatch reported 0 errors, 0 warnings,
and 0 checks, but there is an unnecessary blank line in the generated
patch that I missed. I will remove it.

> Why the prototype right before the definition? Did a LLM create this
> patch?

Yes. The initial implementation was generated by an LLM. The prototype
immediately before pmcraid_complete_reset_cmd() is unnecessary and will
be removed.

I will continue reworking the completion path, reset state machine, list
ownership, and teardown synchronization. The current v1 should not be
reused or resent.

Thanks,
Runyu Xiao

Re: [PATCH] scsi: pmcraid: shut down command timers before reuse

[Greg Kroah-Hartman]

On Fri, Aug 21, 2026 at 04:19:02PM +0800, Runyu Xiao wrote:
> On Fri, Aug 21, 2026 at 02:23:51PM +0800, Greg Kroah-Hartman wrote:
> > How was this found and tested?
> 
> The issue was found by a static-analysis check and then confirmed by
> tracing the command ownership and timer lifetime in pmcraid.c. The v1
> patch was generated by an LLM from that finding.

Then you need to use the Assisted-by tag.

thanks,

greg k-h