Skip to content

Commit 224214e

Browse files
nprao1gregkh
authored andcommitted
pds_core: fix use-after-free on workqueue during remove
[ Upstream commit 0ad1348 ] 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: 01ba61b ("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> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 90d9f3e commit 224214e

2 files changed

Lines changed: 13 additions & 6 deletions

File tree

drivers/net/ethernet/amd/pds_core/core.c

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ static void pdsc_qcq_intr_free(struct pdsc *pdsc, struct pdsc_qcq *qcq)
110110
return;
111111

112112
pdsc_intr_free(pdsc, qcq->intx);
113-
qcq->intx = PDS_CORE_INTR_INDEX_NOT_ASSIGNED;
114113
}
115114

116115
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)
145144

146145
pdsc_qcq_intr_free(pdsc, qcq);
147146

147+
/* Drain any work queued by ISR before it was freed above */
148+
if (qcq->work.func)
149+
cancel_work_sync(&qcq->work);
150+
151+
qcq->intx = PDS_CORE_INTR_INDEX_NOT_ASSIGNED;
152+
148153
if (qcq->q_base)
149154
dma_free_coherent(dev, qcq->q_size,
150155
qcq->q_base, qcq->q_base_pa);
@@ -304,8 +309,11 @@ int pdsc_qcq_alloc(struct pdsc *pdsc, unsigned int type, unsigned int index,
304309

305310
static void pdsc_core_uninit(struct pdsc *pdsc)
306311
{
307-
pdsc_qcq_free(pdsc, &pdsc->notifyqcq);
312+
/* Free adminqcq first: its work accesses notifyqcq, so we must
313+
* disable its IRQ and drain its work before freeing notifyqcq.
314+
*/
308315
pdsc_qcq_free(pdsc, &pdsc->adminqcq);
316+
pdsc_qcq_free(pdsc, &pdsc->notifyqcq);
309317

310318
if (pdsc->kern_dbpage) {
311319
iounmap(pdsc->kern_dbpage);
@@ -472,8 +480,6 @@ void pdsc_teardown(struct pdsc *pdsc, bool removing)
472480
{
473481
if (!pdsc->pdev->is_virtfn)
474482
pdsc_devcmd_reset(pdsc);
475-
if (pdsc->adminqcq.work.func)
476-
cancel_work_sync(&pdsc->adminqcq.work);
477483

478484
pdsc_core_uninit(pdsc);
479485

drivers/net/ethernet/amd/pds_core/main.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -429,15 +429,16 @@ static void pdsc_remove(struct pci_dev *pdev)
429429
pdsc_sriov_configure(pdev, 0);
430430

431431
timer_shutdown_sync(&pdsc->wdtimer);
432-
if (pdsc->wq)
433-
destroy_workqueue(pdsc->wq);
434432

435433
mutex_lock(&pdsc->config_lock);
436434
set_bit(PDSC_S_STOPPING_DRIVER, &pdsc->state);
437435

438436
pdsc_stop(pdsc);
439437
pdsc_teardown(pdsc, PDSC_TEARDOWN_REMOVING);
440438
mutex_unlock(&pdsc->config_lock);
439+
440+
if (pdsc->wq)
441+
destroy_workqueue(pdsc->wq);
441442
mutex_destroy(&pdsc->config_lock);
442443
mutex_destroy(&pdsc->devcmd_lock);
443444

0 commit comments

Comments
 (0)