pds_core: fix use-after-free on workqueue during remove

In pdsc_remove(), the workqueue is destroyed before pdsc_teardown()
is called. This ordering allows two paths to queue work on the
destroyed workqueue:

1. If pdsc_teardown() -> pdsc_devcmd_reset() times out, the error
   path in pdsc_devcmd_locked() queues health_work.

2. A NotifyQ event can trigger the ISR and queue work before free_irq()
   is called in pdsc_teardown().

Fix by moving destroy_workqueue() after pdsc_teardown() so the
workqueue outlives every queuer; destroy_workqueue() then flushes any
work still pending.

Draining the queued work also requires ordering the teardown so the
resources that work touches are freed last:

  - In pdsc_qcq_free(), after freeing the interrupt, cancel_work_sync()
    the queue's work and only then clear qcq->intx, so
    pdsc_process_adminq()'s read of qcq->intx for interrupt-credit
    return cannot race with the clear.

  - Free adminqcq before notifyqcq: the shared adminq ISR is released
    when adminqcq is freed, and the adminq work accesses notifyqcq, so
    both must be stopped before notifyqcq is freed.

Fixes: 01ba61b55b20 ("pds_core: Add adminq processing and commands")
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://patchwork.kernel.org/comment/27002369/
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
Link: https://patch.msgid.link/20260714180223.1642792-3-nikhil.rao@amd.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/drivers/net/ethernet/amd/pds_core/core.c b/drivers/net/ethernet/amd/pds_core/core.c
index 1074a022a52fb4a1db7d62470c59c4ba4cf3b0a3..e39b2c9beb2093c1e03ba723112831835c379cb7 100644 (file)
--- a/drivers/net/ethernet/amd/pds_core/core.c
+++ b/drivers/net/ethernet/amd/pds_core/core.c
@@ -110,7 +110,6 @@ static void pdsc_qcq_intr_free(struct pdsc *pdsc, struct pdsc_qcq *qcq)
                return;
 
        pdsc_intr_free(pdsc, qcq->intx);
-       qcq->intx = PDS_CORE_INTR_INDEX_NOT_ASSIGNED;
 }
 
 static int pdsc_qcq_intr_alloc(struct pdsc *pdsc, struct pdsc_qcq *qcq)
@@ -145,6 +144,12 @@ void pdsc_qcq_free(struct pdsc *pdsc, struct pdsc_qcq *qcq)
 
        pdsc_qcq_intr_free(pdsc, qcq);
 
+       /* Drain any work queued by ISR before it was freed above */
+       if (qcq->work.func)
+               cancel_work_sync(&qcq->work);
+
+       qcq->intx = PDS_CORE_INTR_INDEX_NOT_ASSIGNED;
+
        if (qcq->q_base)
                dma_free_coherent(dev, qcq->q_size,
                                  qcq->q_base, qcq->q_base_pa);
@@ -304,8 +309,11 @@ err_out:
 
 static void pdsc_core_uninit(struct pdsc *pdsc)
 {
-       pdsc_qcq_free(pdsc, &pdsc->notifyqcq);
+       /* Free adminqcq first: its work accesses notifyqcq, so we must
+        * disable its IRQ and drain its work before freeing notifyqcq.
+        */
        pdsc_qcq_free(pdsc, &pdsc->adminqcq);
+       pdsc_qcq_free(pdsc, &pdsc->notifyqcq);
 
        if (pdsc->kern_dbpage) {
                iounmap(pdsc->kern_dbpage);
@@ -479,8 +487,6 @@ void pdsc_teardown(struct pdsc *pdsc, bool removing)
 {
        if (!pdsc->pdev->is_virtfn)
                pdsc_devcmd_reset(pdsc);
-       if (pdsc->adminqcq.work.func)
-               cancel_work_sync(&pdsc->adminqcq.work);
 
        pci_clear_master(pdsc->pdev);
 

diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c
index 22db78343eb075ed96ceb31c413c3c6bb52b7937..638b9c7a509d9d6c5fc431dff1af4887d5c9a1b6 100644 (file)
--- a/drivers/net/ethernet/amd/pds_core/main.c
+++ b/drivers/net/ethernet/amd/pds_core/main.c
@@ -435,8 +435,6 @@ static void pdsc_remove(struct pci_dev *pdev)
                pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev);
 
                timer_shutdown_sync(&pdsc->wdtimer);
-               if (pdsc->wq)
-                       destroy_workqueue(pdsc->wq);
 
                mutex_lock(&pdsc->config_lock);
                set_bit(PDSC_S_STOPPING_DRIVER, &pdsc->state);
@@ -444,6 +442,9 @@ static void pdsc_remove(struct pci_dev *pdev)
                pdsc_stop(pdsc);
                pdsc_teardown(pdsc, PDSC_TEARDOWN_REMOVING);
                mutex_unlock(&pdsc->config_lock);
+
+               if (pdsc->wq)
+                       destroy_workqueue(pdsc->wq);
                mutex_destroy(&pdsc->config_lock);
                mutex_destroy(&pdsc->devcmd_lock);