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

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

[Pei Xiao]

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: stable@vger.kernel.org
Assisted-by: Codex:deepseek-v4-flash
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
changes in v3:
1.Add Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
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/5ce42824-488e-4040-8531-0acc8b01b13a@deltatee.com/#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: stable@vger.kernel.org
---
 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-After-Free window in the `switchtec_pci_probe()` error path due to premature freeing of `stdev` while `event_irq` is active.
- [High] Use-After-Free via TOCTOU race in `switchtec_dev_write` due to incorrect teardown ordering in `stdev_kill()`.
--

--- Patch [1]: [PATCH v3] PCI: switchtec: Fix use-after-free in switchtec_pci_remove due to race condition ---
commit a18ebaceba03ea5f94a521c5ced89b052f121ea9
Author: Pei Xiao <xiaopei01@kylinos.cn>
Subject: PCI: switchtec: Fix use-after-free in switchtec_pci_remove due to race condition

This patch attempts to fix a use-after-free bug during device removal by
quiescing interrupt sources before canceling pending work items. It adds
logic to stdev_kill() to explicitly free IRQs and cancel work structures.
It also stores the IRQ numbers explicitly in the switchtec_dev structure.

Link: https://lore.kernel.org/lkml/5ce42824-488e-4040-8531-0acc8b01b13a@deltatee.com/#t

> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 41fc4b5127081..91d44acfe1193 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);
[Severity: High]
This is a pre-existing issue, but does canceling this delayed work before
setting the alive flag allow it to be re-armed?

If userspace performs a concurrent write during device removal, it can call
switchtec_dev_write(), which does:

drivers/pci/switch/switchtec.c:switchtec_dev_write() {
    ...
    rc = lock_mutex_and_test_alive(stdev);
    if (rc)
        return rc;
    ...
    rc = mrpc_queue_cmd(stuser);
    ...
}

Because cancel_delayed_work_sync() executes outside of mrpc_mutex and before
stdev->alive is set to false, mrpc_queue_cmd() could successfully re-arm
stdev->mrpc_timeout after it was just canceled.

When stdev is subsequently freed, could this newly armed delayed work execute
on freed memory?

>  
>  	/* Mark the hardware as unavailable and complete all completions */

[ ... ]

> @@ -1513,6 +1522,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
>  
>  	if (rc)
>  		return rc;
> +	stdev->event_irq = event_irq;
[Severity: High]
This is a pre-existing issue, but is there a use-after-free window in the
probe error path?

When switchtec_init_isr() requests event_irq, it uses devm_request_irq()
bound to the PCI device:

drivers/pci/switch/switchtec.c:switchtec_init_isr() {
    ...
    rc = devm_request_irq(&stdev->pdev->dev, event_irq,
                          switchtec_event_isr, 0, KBUILD_MODNAME, stdev);
    ...
}

If a subsequent initialization step fails during switchtec_pci_probe(),
the err_put path will free the stdev structure:

drivers/pci/switch/switchtec.c:switchtec_pci_probe() {
    ...
err_put:
    ida_free(&switchtec_minor_ida, MINOR(stdev->dev.devt));
    put_device(&stdev->dev);
    return rc;
}

Since put_device() immediately frees stdev without explicitly freeing the IRQ,
and devres cleanup doesn't run until the probe function returns, could an
interrupt fire during this window and cause switchtec_event_isr() to
dereference the already freed stdev?

>  
>  	if (!stdev->dma_mrpc)
>  		return rc;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4cebfe41ee3985bc4f38beb42d44147a34637971.1786944920.git.xiaopei01@kylinos.cn?part=1

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

[Pei Xiao]

在 2026/8/17 13:53, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Use-After-Free window in the `switchtec_pci_probe()` error path due to premature freeing of `stdev` while `event_irq` is active.
> - [High] Use-After-Free via TOCTOU race in `switchtec_dev_write` due to incorrect teardown ordering in `stdev_kill()`.
> --
> 
> --- Patch [1]: [PATCH v3] PCI: switchtec: Fix use-after-free in switchtec_pci_remove due to race condition ---
> commit a18ebaceba03ea5f94a521c5ced89b052f121ea9
> Author: Pei Xiao <xiaopei01@kylinos.cn>
> Subject: PCI: switchtec: Fix use-after-free in switchtec_pci_remove due to race condition
> 
> This patch attempts to fix a use-after-free bug during device removal by
> quiescing interrupt sources before canceling pending work items. It adds
> logic to stdev_kill() to explicitly free IRQs and cancel work structures.
> It also stores the IRQ numbers explicitly in the switchtec_dev structure.
> 
> Link: https://lore.kernel.org/lkml/5ce42824-488e-4040-8531-0acc8b01b13a@deltatee.com/#t
> 
>> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
>> index 41fc4b5127081..91d44acfe1193 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);
> [Severity: High]
> This is a pre-existing issue, but does canceling this delayed work before
> setting the alive flag allow it to be re-armed?
> 
> If userspace performs a concurrent write during device removal, it can call
> switchtec_dev_write(), which does:
> 
> drivers/pci/switch/switchtec.c:switchtec_dev_write() {
>     ...
>     rc = lock_mutex_and_test_alive(stdev);
>     if (rc)
>         return rc;
>     ...
>     rc = mrpc_queue_cmd(stuser);
>     ...
> }
> 
> Because cancel_delayed_work_sync() executes outside of mrpc_mutex and before
> stdev->alive is set to false, mrpc_queue_cmd() could successfully re-arm
> stdev->mrpc_timeout after it was just canceled.
> 
> When stdev is subsequently freed, could this newly armed delayed work execute
> on freed memory?
I have send this patch for this bug:
https://lore.kernel.org/lkml/e640c79072065c1a347ed25f7ce0fe1ba565f4fe.1786017236.git.xiaopei01@kylinos.cn/
> 
>>  
>>  	/* Mark the hardware as unavailable and complete all completions */
> 
> [ ... ]
> 
>> @@ -1513,6 +1522,7 @@ static int switchtec_init_isr(struct switchtec_dev *stdev)
>>  
>>  	if (rc)
>>  		return rc;
>> +	stdev->event_irq = event_irq;
> [Severity: High]
> This is a pre-existing issue, but is there a use-after-free window in the
> probe error path?
> 
> When switchtec_init_isr() requests event_irq, it uses devm_request_irq()
> bound to the PCI device:
> 
> drivers/pci/switch/switchtec.c:switchtec_init_isr() {
>     ...
>     rc = devm_request_irq(&stdev->pdev->dev, event_irq,
>                           switchtec_event_isr, 0, KBUILD_MODNAME, stdev);
>     ...
> }
> 
> If a subsequent initialization step fails during switchtec_pci_probe(),
> the err_put path will free the stdev structure:
> 
> drivers/pci/switch/switchtec.c:switchtec_pci_probe() {
>     ...
> err_put:
>     ida_free(&switchtec_minor_ida, MINOR(stdev->dev.devt));
>     put_device(&stdev->dev);
>     return rc;
> }
> 
> Since put_device() immediately frees stdev without explicitly freeing the IRQ,
> and devres cleanup doesn't run until the probe function returns, could an
> interrupt fire during this window and cause switchtec_event_isr() to
> dereference the already freed stdev?
> 
>>  
>>  	if (!stdev->dma_mrpc)
>>  		return rc;
> 
> [ ... ]
>