Re: [PATCH net] pds_core: fix use-after-free on workqueue during remove
From: Simon Horman <horms@kernel.org>
Date: 2026-06-11 17:33:18
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. Full review at: https://sashiko.dev --- pds_core: fix use-after-free on workqueue during remove This commit fixes a use-after-free on the workqueue during remove by destroying the workqueue earlier and setting pdsc->wq to NULL.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/amd/pds_core/main.c b/drivers/net/ethernet/amd/pds_core/main.c index 22db78343eb07..cb3ac26266aa7 100644 --- a/drivers/net/ethernet/amd/pds_core/main.c +++ b/drivers/net/ethernet/amd/pds_core/main.c@@ -435,8 +435,10 @@ static void pdsc_remove(struct pci_dev *pdev) pdsc_auxbus_dev_del(pdsc, pdsc, &pdsc->padev); timer_shutdown_sync(&pdsc->wdtimer); - if (pdsc->wq) + if (pdsc->wq) { destroy_workqueue(pdsc->wq);
[Severity: High] This is a pre-existing issue, but can this destroy_workqueue() call deadlock with pdsc_pci_reset_thread()? The PCI subsystem holds device_lock() when calling pdsc_remove(). If pdsc_pci_reset_thread() is currently executing on this workqueue, destroy_workqueue() will block waiting for it to finish. However, pdsc_pci_reset_thread() calls pci_reset_function(), which attempts to acquire the same device_lock(), resulting in a deadlock.
+ pdsc->wq = NULL;
[Severity: High]
Can setting pdsc->wq to NULL here introduce a regression where the interrupt
handler dereferences a NULL pointer?
This assignment is executed before set_bit(PDSC_S_STOPPING_DRIVER,
&pdsc->state) and before pdsc_stop() masks hardware interrupts.
If an interrupt fires (e.g., a NotifyQ event) in this window,
pdsc_adminq_isr() will run. Because the PDSC_S_STOPPING_DRIVER state is not
yet set, the ISR check succeeds and it will unconditionally call queue_work()
with the NULL pdsc->wq:
pdsc_adminq_isr()
...
queue_work(pdsc->wq, &qcq->work);
+ } mutex_lock(&pdsc->config_lock); set_bit(PDSC_S_STOPPING_DRIVER, &pdsc->state);