[PATCH v2] scsi: smartpqi: drain controller workers before freeing controller

[Fan Wu]

pqi_remove_ctrl() stops the heartbeat timer and cancels the rescan and
update_time delayed workers, but the controller can still have
ctrl_offline_work, event_work, ofa_memory_alloc_work and ofa_quiesce_work
queued (armed from the heartbeat timer and IRQ/error paths). These recover
struct pqi_ctrl_info through container_of and dereference it, so a worker
that runs after pqi_free_ctrl_info() frees the controller is a
use-after-free.

Drain them in pqi_free_ctrl_resources(). pqi_event_worker() starts by
wait_event()ing on block_requests, which pqi_remove_ctrl() sets and never
clears in the remove path, so cancelling it while blocked would wait
forever; unblock first. The controller is already offline by then
(controller_online was cleared earlier in pqi_remove_ctrl()), so the
released worker returns without rearming rescan or OFA work.

ctrl_offline_work is disabled rather than cancelled because
pqi_take_ctrl_offline() can re-queue it. Its callback also calls
pqi_free_interrupts(), so it must be drained before the teardown's own
pqi_free_interrupts() to avoid a concurrent double free_irq() on the same
MSIX vectors.

After the IRQs are freed and event_work is drained, cancel the rescan
worker again: an event worker that passed the pqi_ctrl_offline() check
before the controller went offline may still have scheduled rescan via
pqi_schedule_rescan_worker_with_delay(). The OFA workers are cancelled
last; their sole producer is event_work, already drained.

This bug was found by static analysis.

Fixes: 5f310425c8ea ("scsi: smartpqi: update rescan worker")
Cc: stable@xxxxxxxxxxxxxxx # needs adjustments for <= 6.6: use cancel_work_sync()
Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>

---

Changes from v1:
- Fixed deadlock: cancel_work_sync(event_work) while block_requests was
  set could wait forever; now unblocks first.
- Moved disable_work_sync(ctrl_offline_work) before pqi_free_interrupts()
  to avoid concurrent double free_irq() with the offline worker's own
  interrupt freeing.
- Cancel rescan_work again after draining event_work, to catch a rescan
  armed by an event worker that passed the offline check before the
  controller went offline.
---
 drivers/scsi/smartpqi/smartpqi_init.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index fe549e2b7c94..14f85fb95dfa 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -8891,7 +8891,19 @@ static void pqi_free_interrupts(struct pqi_ctrl_info *ctrl_info)
 
 static void pqi_free_ctrl_resources(struct pqi_ctrl_info *ctrl_info)
 {
+	/*
+	 * Release blocked workers first.  Disable ctrl_offline_work before
+	 * freeing IRQs because its callback can also free them.  event_work
+	 * can requeue rescan_work, so drain event_work before cancelling
+	 * rescan again.
+	 */
+	pqi_ctrl_unblock_requests(ctrl_info);
+	disable_work_sync(&ctrl_info->ctrl_offline_work);
 	pqi_free_interrupts(ctrl_info);
+	cancel_work_sync(&ctrl_info->event_work);
+	pqi_cancel_rescan_worker(ctrl_info);
+	cancel_work_sync(&ctrl_info->ofa_memory_alloc_work);
+	cancel_work_sync(&ctrl_info->ofa_quiesce_work);
 	if (ctrl_info->queue_memory_base)
 		dma_free_coherent(&ctrl_info->pci_dev->dev,
 			ctrl_info->queue_memory_length,
-- 
2.34.1