[PATCH] scsi: dc395x: sync the waiting_timer before freeing the host

[PATCH] scsi: dc395x: sync the waiting_timer before freeing the host

[Fan Wu]

The waiting_timer callback re-arms itself and takes the host lock.
adapter_uninit() currently stops it with timer_delete(), which does not
wait for a callback already running on another CPU. The final
scsi_host_put() that frees the host (and the host lock the callback
dereferences) runs right after adapter_uninit(), so a concurrent or just
re-armed callback can fire after the free, leaving a potential
use-after-free window.

Replace timer_delete() with timer_shutdown_sync() and move it after
free_irq(), outside the host-lock section. This waits for a running
callback and prevents it from re-arming, while avoiding the self-deadlock
that would result from syncing a callback blocked on the lock we still
hold.

selto_timer is left untouched; it has no reachable armer or callback.

This issue was found by an in-house static analysis tool.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Cc: Jamie Lenehan <lenehan@twibble.org>
Assisted-by: Codex:gpt-5.6
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---
 drivers/scsi/dc395x.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 6183ce05d..3e85d422c 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3816,18 +3816,14 @@ static void adapter_uninit(struct AdapterCtlBlk *acb)
 	unsigned long flags;
 	DC395x_LOCK_IO(acb->scsi_host, flags);
 
-	/* remove timers */
-	if (timer_pending(&acb->waiting_timer))
-		timer_delete(&acb->waiting_timer);
-	if (timer_pending(&acb->selto_timer))
-		timer_delete(&acb->selto_timer);
-
 	adapter_uninit_chip(acb);
 	adapter_remove_and_free_all_devices(acb);
 	DC395x_UNLOCK_IO(acb->scsi_host, flags);
 
 	if (acb->irq_level)
 		free_irq(acb->irq_level, acb);
+	/* waiting_timer self-rearms and takes host_lock: sync outside lock. */
+	timer_shutdown_sync(&acb->waiting_timer);
 	if (acb->io_port_base)
 		release_region(acb->io_port_base, acb->io_port_len);
 

Re: [PATCH] scsi: dc395x: sync the waiting_timer before freeing the host

[Oliver Neukum]

On 10.08.26 07:50, Fan Wu wrote:
> The waiting_timer callback re-arms itself and takes the host lock.
> adapter_uninit() currently stops it with timer_delete(), which does not
> wait for a callback already running on another CPU. The final
> scsi_host_put() that frees the host (and the host lock the callback
> dereferences) runs right after adapter_uninit(), so a concurrent or just
> re-armed callback can fire after the free, leaving a potential
> use-after-free window.
> 
> Replace timer_delete() with timer_shutdown_sync() and move it after
> free_irq(), outside the host-lock section. This waits for a running
> callback and prevents it from re-arming, while avoiding the self-deadlock
> that would result from syncing a callback blocked on the lock we still
> hold.

Thank you for the patch. It raises a question, though. Do you want
the timers to fire after the adapter has already ceased to function?

	Regards
		Oliver