[PATCH] scsi: smartpqi: cancel pending workers before freeing controller

[Fan Wu]

The smartpqi driver schedules four struct work_struct members of
struct pqi_ctrl_info on the system workqueue: ctrl_offline_work,
event_work, ofa_memory_alloc_work and ofa_quiesce_work.
ctrl_offline_work is armed by pqi_take_ctrl_offline(), reached from the
heartbeat timer handler and the error paths; event_work is queued by the
event-interrupt handler, and the event worker can in turn queue the two
OFA workers while processing OFA events.

None of these workers is cancelled during controller teardown.
pqi_remove_ctrl() stops the heartbeat timer and cancels the rescan and
update_time delayed workers, then calls pqi_free_ctrl_resources(), which
frees the interrupts and finally the controller in pqi_free_ctrl_info()
(kfree).  A worker queued by the timer or the event handler can therefore
run after the controller has been freed:

    CPU0 (teardown)                 CPU1 (system_wq)
    heartbeat timer / event IRQ
    pqi_take_ctrl_offline()
      schedule_work(ctrl_offline_work)
    pqi_remove_ctrl()
      ...
      pqi_free_ctrl_resources()
        free_irq()
        kfree(ctrl_info)            pqi_ctrl_offline_worker()
                                      container_of(work) -> freed ctrl_info
                                      pqi_take_ctrl_offline_deferred()
                                        dereferences ctrl_info  UAF

Clearing controller_online does not stop an already-queued
ctrl_offline_work: pqi_ctrl_offline_worker() and its callee
pqi_take_ctrl_offline_deferred() never check it and dereference the
controller unconditionally.  pqi_event_worker() and the two OFA workers
recover the controller with container_of() and dereference it the same way.

Drain the workers in pqi_free_ctrl_resources(), in an order that drains
each layer of the arming chain.

ctrl_offline_work is disabled with disable_work_sync() rather than
cancelled.  Its armer, pqi_take_ctrl_offline(), checks controller_online
and then, still in the same call, clears it and calls schedule_work(); an
interrupt or error handler that has already passed that check will queue
ctrl_offline_work even after pqi_remove_ctrl() has cleared
controller_online, so a plain cancel_work_sync() can return before that
in-flight armer queues the work.  The worker's callback,
pqi_take_ctrl_offline_deferred(), also calls pqi_free_interrupts(), so an
instance running during teardown would race the
num_msix_vectors_initialized counter in pqi_free_irqs() and call
free_irq() twice for the same vectors.  disable_work_sync() drains any
queued or running instance and makes later schedule_work() calls on it
no-ops, so the worker can neither run during the teardown's own
pqi_free_interrupts() nor survive until the controller is freed.

The remaining three workers are cancelled after pqi_free_interrupts().
Freeing the IRQs first stops the handler from arming event_work, draining
event_work next stops the event worker from arming the OFA workers, and
the OFA workers are drained last: each cancel is a final drain with no
re-arm race.

cancel_work_sync() is the correct primitive for the remaining work_struct
items: it cancels a pending instance and waits for a running callback to
return.  pqi_free_ctrl_resources() is reached only through the
pqi_remove_ctrl() chokepoint shared by pqi_pci_remove() and the probe
error path, and the four work_structs are initialised in
pqi_alloc_ctrl_info(), so draining here always operates on initialised
work.

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>
---
 drivers/scsi/smartpqi/smartpqi_init.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -8891,7 +8891,16 @@ static void pqi_free_interrupts(struct pqi_ctrl_info *ctrl_info)

 static void pqi_free_ctrl_resources(struct pqi_ctrl_info *ctrl_info)
 {
+	/*
+	 * Disable, not cancel: the callback also frees IRQs, and
+	 * pqi_take_ctrl_offline() can re-queue it after a cancel.
+	 */
+	disable_work_sync(&ctrl_info->ctrl_offline_work);
 	pqi_free_interrupts(ctrl_info);
+	/* IRQs queue event_work, which can queue OFA work. */
+	cancel_work_sync(&ctrl_info->event_work);
+	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,