All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] scsi: pmcraid: shut down command timers before reuse
@ 2026-08-21  6:23 Runyu Xiao
  2026-08-21  6:45 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Runyu Xiao @ 2026-08-21  6:23 UTC (permalink / raw)
  To: James E . J . Bottomley
  Cc: Martin K . Petersen, linux-scsi, linux-kernel, stable, Runyu Xiao,
	Jianhao Xu

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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: pmcraid: shut down command timers before reuse
  2026-08-21  6:23 [PATCH] scsi: pmcraid: shut down command timers before reuse Runyu Xiao
@ 2026-08-21  6:45 ` sashiko-bot
  2026-08-21  7:00 ` Greg KH
  2026-08-21  8:19 ` Runyu Xiao
  2 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-08-21  6:45 UTC (permalink / raw)
  To: Runyu Xiao; +Cc: linux-scsi

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

New issues:
- [High] Massive performance regression for all SCSI I/O due to unconditional workqueue deferral.
- [High] Adapter reset state machine corruption causes premature failure via double-increment of retry counter.
- [High] Calling `timer_shutdown()` asynchronously stalls the adapter reset sequence permanently.

Pre-existing issues:
- [Critical] List corruption and kernel panic due to a double `list_del()` race between tasklet and timeout handlers.
- [Critical] Lockless TOCTOU race on `pinstance->reset_cmd` causes Use-After-Free and memory corruption.
- [High] Freeing command blocks during teardown without synchronizing timers causes Use-After-Free.
--

--- Patch [1]: [PATCH] scsi: pmcraid: shut down command timers before reuse ---
commit 1fe43c3a7e2ec130d4b5a6182060aafaaff8b4a8
Author: Runyu Xiao <runyu.xiao@seu.edu.cn>

scsi: pmcraid: shut down command timers before reuse

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

> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index 942a993932049..ed2ebe68c816b 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c

[ ... ]

> @@ -55,6 +55,43 @@ static unsigned int pmcraid_enable_msix;

[ ... ]

> +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);
> +}

[Severity: High]
Does calling timer_shutdown() permanently break the reset engine?

timer_shutdown() permanently disables the timer. If a command times out,
pmcraid_timeout_handler() starts the reset sequence by re-using the timed-out
cmd block and calling add_timer() to rearm it.

Concurrently, if the tasklet processes a late response for this command, it
calls pmcraid_schedule_cmd_work(), which executes timer_shutdown().

This makes the reset engine's add_timer() a silent no-op.

Will the reset sequence hang forever waiting for this cancelled timer?

> @@ -483,8 +521,6 @@ static void pmcraid_clr_trans_op(
>  	struct pmcraid_instance *pinstance
>  )
>  {

[ ... ]

>  	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);
>  	}
>  }

[Severity: Critical]
This is a pre-existing issue, but does accessing pinstance->reset_cmd outside
the host_lock introduce a use-after-free?

In pmcraid_clr_trans_op(), pinstance->reset_cmd is evaluated and dereferenced
without holding pinstance->host->host_lock.

Concurrently, pmcraid_ioa_reset() (which holds the lock) can complete the
reset, set pinstance->reset_cmd = NULL, and return the command to the free
pool.

If a transition-to-operational interrupt races with reset completion, could
pmcraid_schedule_cmd_work() be called on a stale pointer, corrupting
cmd->work_fn of a freed command?

> @@ -1998,42 +2057,28 @@ static void pmcraid_fail_outstanding_cmds(struct pmcraid_instance *pinstance)

[ ... ]

>  		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;

[Severity: High]
Will this cause the retry counter to be double-incremented for a single reset
attempt?

During IOA_STATE_IN_HARD_RESET, pmcraid_ioa_reset() increments
pinstance->ioa_reset_attempts.

If the reset command is pending, pmcraid_fail_outstanding_cmds() defers its
completion and returns true, causing pmcraid_ioa_reset() to break out of the
switch and return early.

When the worker later runs pmcraid_complete_reset_cmd(), it calls cmd_done()
(which is pmcraid_ioa_reset()) again. Since the state is still
IOA_STATE_IN_HARD_RESET, ioa_reset_attempts will be incremented a second time
for the exact same physical reset attempt.

Could this mark the adapter as dead prematurely?

> @@ -3982,17 +4029,15 @@ static void pmcraid_tasklet_function(unsigned long instance)

[ ... ]

>  		spin_lock_irqsave(&pinstance->pending_pool_lock,
>  				   pending_lock_flags);
>  		list_del(&cmd->free_list);
>  		spin_unlock_irqrestore(&pinstance->pending_pool_lock,
>  					pending_lock_flags);

[Severity: Critical]
This is a pre-existing issue, but is it possible to corrupt the list here?

If a command times out, pmcraid_timeout_handler() starts a reset and
pmcraid_fail_outstanding_cmds() iteratively unlinks all commands from
pending_cmd_pool using list_del() under pending_pool_lock.

Concurrently, if the response was already placed in the HRRQ by the adapter,
pmcraid_tasklet_function() processes it, acquires pending_pool_lock, and
blindly calls list_del().

If the command was already removed by the reset path, wouldn't this second
list_del() corrupt the list?

> -		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);

[Severity: High]
Does this code defer every normal SCSI completion to a workqueue?

In pmcraid_tasklet_function(), normal SCSI I/O operations have cmd->cmd_done
set to pmcraid_io_done.

By routing every response with cmd->cmd_done != NULL to
pmcraid_schedule_cmd_work(), does every single SCSI read and write incur a
context switch to system_percpu_wq?

Will this cause a severe latency and CPU overhead regression for all block
I/O?

> @@ -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);

[Severity: High]
This is a pre-existing issue, but does this function free command blocks
without shutting down their timers?

During adapter teardown or module unload, pmcraid_release_cmd_blocks()
iterates over the command blocks and frees them via kmem_cache_free().

It flushes the async workqueue but does not call timer_shutdown_sync().

If any command timer is still pending or executing, will the timer subsystem
access the freed command block and cause a use-after-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821062351.72658-1-runyu.xiao@seu.edu.cn?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: pmcraid: shut down command timers before reuse
  2026-08-21  6:23 [PATCH] scsi: pmcraid: shut down command timers before reuse Runyu Xiao
  2026-08-21  6:45 ` sashiko-bot
@ 2026-08-21  7:00 ` Greg KH
  2026-08-21  8:19 ` Runyu Xiao
  2 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2026-08-21  7:00 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: James E . J . Bottomley, Martin K . Petersen, linux-scsi,
	linux-kernel, stable, Jianhao Xu

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: pmcraid: shut down command timers before reuse
  2026-08-21  6:23 [PATCH] scsi: pmcraid: shut down command timers before reuse Runyu Xiao
  2026-08-21  6:45 ` sashiko-bot
  2026-08-21  7:00 ` Greg KH
@ 2026-08-21  8:19 ` Runyu Xiao
  2026-08-21 11:01   ` Greg Kroah-Hartman
  2 siblings, 1 reply; 5+ messages in thread
From: Runyu Xiao @ 2026-08-21  8:19 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: James E . J . Bottomley, Martin K . Petersen, linux-scsi,
	linux-kernel, stable, Runyu Xiao, Jianhao Xu

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: pmcraid: shut down command timers before reuse
  2026-08-21  8:19 ` Runyu Xiao
@ 2026-08-21 11:01   ` Greg Kroah-Hartman
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-21 11:01 UTC (permalink / raw)
  To: Runyu Xiao
  Cc: James E . J . Bottomley, Martin K . Petersen, linux-scsi,
	linux-kernel, stable, Jianhao Xu

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-08-21 11:03 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21  6:23 [PATCH] scsi: pmcraid: shut down command timers before reuse Runyu Xiao
2026-08-21  6:45 ` sashiko-bot
2026-08-21  7:00 ` Greg KH
2026-08-21  8:19 ` Runyu Xiao
2026-08-21 11:01   ` Greg Kroah-Hartman

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.