github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-27 17:47:41 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

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

Browse Source 
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: 01ba61b55b ("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>
This commit is contained in:
Nikhil P. Rao 2026-07-14 18:02:23 +00:00 committed by Jakub Kicinski
parent ab0eec0ff0
commit 0ad1348815
2 changed files with 13 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
drivers/net/ethernet/amd/pds_core/core.c
View File

@ -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 @@ int pdsc_qcq_alloc(struct pdsc *pdsc, unsigned int type, unsigned int index,
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);

5
drivers/net/ethernet/amd/pds_core/main.c
View File

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