[PATCH v3] PCI: switchtec: Fix use-after-free in switchtec_pci_remove due to race condition

Pei Xiao <[email protected]>
Newsgroups org.kernel.vger.linux-pci,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <4cebfe41ee3985bc4f38beb42d44147a34637971.1786944920.git.xiaopei01@kylinos.cn>
In stdev_create, &stdev->mrpc_work is bound with mrpc_event_work, and
&stdev->link_event_work is bound with link_event_work. The IRQ handlers
switchtec_event_isr and switchtec_dma_mrpc_isr can schedule these works
on system_wq (via schedule_work() in the ISRs and via
check_link_state_events()).

If we remove the device, switchtec_pci_remove makes cleanup and the
memory allocated for stdev is released by put_device() ->
stdev_release() -> kfree(stdev), while the works mentioned above may
still be pending or running. The sequence of operations that may lead
to a UAF bug is as follows:

CPU0                                      CPU1

                                          | switchtec_event_isr
                                          | schedule_work(&stdev->mrpc_work)
switchtec_pci_remove                      |
cdev_device_del(&stdev->cdev,             |
                &stdev->dev)              |
stdev_kill(stdev)                         |
switchtec_exit_pci(stdev)                 |
pci_dev_put(stdev->pdev)                  |
put_device(&stdev->dev)                   |
// stdev_release -> kfree(stdev)          |
                                          | mrpc_event_work
                                          | // use stdev (use-after-free)

Fix it by quiescing the interrupt sources before canceling the works:
stdev_kill() first clears PCI bus mastering, then explicitly frees both
IRQs, so no handler can be running and scheduling new work while the works
are canceled.

Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
Fixes: 48c302dc8f3a ("NTB: switchtec: Add link event notifier callback")
Cc: [email protected]
Assisted-by: Codex:deepseek-v4-flash
Reviewed-by: Logan Gunthorpe <[email protected]>
Signed-off-by: Pei Xiao <[email protected]>
---
changes in v3:
1.Add Reviewed-by: Logan Gunthorpe <[email protected]>
2.event_irq and dma_mrpc_irq explictily initialized to zero and check for non-zero in the tests
v2 Links: https://lore.kernel.org/lkml/[email protected]/#t  

changes in v2:
1.Add explicitly devm_free_irq
2.cacel mrpc_work and link_event_work move to before mrpc_timeout
3.Add event_irq and dma_mrpc_irq in struct stdev
4.Add Cc: [email protected]
---
 drivers/pci/switch/switchtec.c | 16 +++++++++++++++-
 include/linux/switchtec.h      |  2 ++
 2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 5711aaa5df11..9eded14a37b1 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1318,6 +1318,13 @@ static void stdev_kill(struct switchtec_dev *stdev)
 
 	pci_clear_master(stdev->pdev);
 
+	if (stdev->event_irq)
+		devm_free_irq(&stdev->pdev->dev, stdev->event_irq, stdev);
+	if (stdev->dma_mrpc_irq)
+		devm_free_irq(&stdev->pdev->dev, stdev->dma_mrpc_irq, stdev);
+
+	cancel_work_sync(&stdev->mrpc_work);
+	cancel_work_sync(&stdev->link_event_work);
 	cancel_delayed_work_sync(&stdev->mrpc_timeout);
 
 	/* Mark the hardware as unavailable and complete all completions */
@@ -1356,6 +1363,8 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev)
 	INIT_LIST_HEAD(&stdev->mrpc_queue);
 	mutex_init(&stdev->mrpc_mutex);
 	stdev->mrpc_busy = 0;
+	stdev->event_irq = 0;
+	stdev->dma_mrpc_irq = 0;
 	INIT_WORK(&stdev->mrpc_work, mrpc_event_work);
 	INIT_DELAYED_WORK(&stdev->mrpc_timeout, mrpc_timeout_work);
 	INIT_WORK(&stdev->link_event_work, link_event_work);
@@ -1513,6 +1522,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 
 	if (rc)
 		return rc;
+	stdev->event_irq = event_irq;
 
 	if (!stdev->dma_mrpc)
 		return rc;
@@ -1529,7 +1539,11 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
 				switchtec_dma_mrpc_isr, 0,
 				KBUILD_MODNAME, stdev);
 
-	return rc;
+	if (rc)
+		return rc;
+	stdev->dma_mrpc_irq = dma_mrpc_irq;
+
+	return 0;
 }
 
 static void init_pff(struct switchtec_dev *stdev)
diff --git a/include/linux/switchtec.h b/include/linux/switchtec.h
index 724da6c08bf7..fd38d3e7f3b6 100644
--- a/include/linux/switchtec.h
+++ b/include/linux/switchtec.h
@@ -500,6 +500,8 @@ struct switchtec_dev {
 	struct mutex mrpc_mutex;
 	struct list_head mrpc_queue;
 	int mrpc_busy;
+	int event_irq;
+	int dma_mrpc_irq;
 	struct work_struct mrpc_work;
 	struct delayed_work mrpc_timeout;
 	bool alive;
-- 
2.25.1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.