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

[PATCH] 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 canceling the works after the sources that can schedule them
have been stopped: stdev_kill() first clears PCI bus mastering, which
prevents the MSI/MSI-X based IRQ handlers from firing and scheduling
new works, and the works are then canceled before the remaining
cleanup and the release of stdev. This also covers the probe error
path, which calls stdev_kill().

Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
Fixes: 48c302dc8f3a ("NTB: switchtec: Add link event notifier callback")
Assisted-by: Codex:deepseek-v4-flash
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/pci/switch/switchtec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 5711aaa5df11..f339ea54aa38 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1319,6 +1319,8 @@ static void stdev_kill(struct switchtec_dev *stdev)
 	pci_clear_master(stdev->pdev);
 
 	cancel_delayed_work_sync(&stdev->mrpc_timeout);
+	cancel_work_sync(&stdev->mrpc_work);
+	cancel_work_sync(&stdev->link_event_work);
 
 	/* Mark the hardware as unavailable and complete all completions */
 	scoped_guard (mutex, &stdev->mrpc_mutex) {
-- 
2.25.1

Re: [PATCH] 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] Incomplete fix for Use-After-Free: `cancel_work_sync()` is called without synchronizing or freeing active IRQ handlers, allowing ISRs to execute after `stdev` is freed and re-schedule works.
- [High] Incorrect cancellation ordering allows `mrpc_timeout` delayed work to be re-armed by concurrent userspace IO after `cancel_delayed_work_sync()`, leading to Use-After-Free.
--

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

This commit attempts to fix a use-after-free bug by canceling the mrpc_work
and link_event_work works during stdev_kill(). It ensures works are canceled
after pci_clear_master() stops the endpoint from issuing further interrupts.

> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 41fc4b5127081..8426d5a9c1c7d 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -1319,6 +1319,8 @@ static void stdev_kill(struct switchtec_dev *stdev)
>  	pci_clear_master(stdev->pdev);
>  
>  	cancel_delayed_work_sync(&stdev->mrpc_timeout);

[Severity: High]
This is a pre-existing issue, but does canceling this delayed work here leave
a race window where concurrent userspace operations can re-arm it?

Since cancel_delayed_work_sync() is called before the mrpc_mutex is acquired
and stdev->alive is set to false, can a concurrent switchtec_dev_write()
verify stdev->alive == true and call mrpc_cmd_submit()?

mrpc_cmd_submit()
  schedule_delayed_work(&stdev->mrpc_timeout, msecs_to_jiffies(500));

If userspace then closes the file descriptor, dropping the final kobject
reference and freeing stdev, won't the delayed work timer expire and execute
mrpc_timeout_work() on the freed stdev?

> +	cancel_work_sync(&stdev->mrpc_work);
> +	cancel_work_sync(&stdev->link_event_work);

[Severity: High]
This isn't a bug introduced by this patch, but does pci_clear_master() wait
for currently executing interrupt handlers to finish?

During device unbind, switchtec_pci_remove() calls stdev_kill(). While
pci_clear_master() prevents new interrupts from being initiated, it doesn't
flush pending interrupts or wait for executing handlers like
switchtec_event_isr().

Could a handler continue running concurrently, and after these works are
canceled, schedule the work again?

switchtec_event_isr()
  schedule_work(&stdev->mrpc_work);

If put_device(&stdev->dev) then frees stdev before the driver's devres (which
owns the interrupt) runs, won't this result in a use-after-free when the
handler places the freed work struct onto the workqueue?

>  
>  	/* Mark the hardware as unavailable and complete all completions */
>  	scoped_guard (mutex, &stdev->mrpc_mutex) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/6fdcbfa869f707cad783c7f92d1681f0a2bd9a7c.1785813784.git.xiaopei01@kylinos.cn?part=1

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

[Logan Gunthorpe]

On 2026-08-03 21:24, Pei Xiao wrote:
> 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 canceling the works after the sources that can schedule them
> have been stopped: stdev_kill() first clears PCI bus mastering, which
> prevents the MSI/MSI-X based IRQ handlers from firing and scheduling
> new works, and the works are then canceled before the remaining
> cleanup and the release of stdev. This also covers the probe error
> path, which calls stdev_kill().
> 
> Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
> Fixes: 48c302dc8f3a ("NTB: switchtec: Add link event notifier callback")
> Assisted-by: Codex:deepseek-v4-flash
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
>  drivers/pci/switch/switchtec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 5711aaa5df11..f339ea54aa38 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -1319,6 +1319,8 @@ static void stdev_kill(struct switchtec_dev *stdev)
>  	pci_clear_master(stdev->pdev);
>  
>  	cancel_delayed_work_sync(&stdev->mrpc_timeout);
> +	cancel_work_sync(&stdev->mrpc_work);
> +	cancel_work_sync(&stdev->link_event_work);

I'm wondering if these should come before the mrpc_timeout sync.
Otherwise, hypothetically, new work could be added and another timeout
could be in progress.

Also, I'm not sure, but seems like the interrupt should be disabled
before this as well?

Thanks!

Logan

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

[Pei Xiao]

在 2026/8/5 03:48, Logan Gunthorpe 写道:
> 
> 
> On 2026-08-03 21:24, Pei Xiao wrote:
>> 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 canceling the works after the sources that can schedule them
>> have been stopped: stdev_kill() first clears PCI bus mastering, which
>> prevents the MSI/MSI-X based IRQ handlers from firing and scheduling
>> new works, and the works are then canceled before the remaining
>> cleanup and the release of stdev. This also covers the probe error
>> path, which calls stdev_kill().
>>
>> Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
>> Fixes: 48c302dc8f3a ("NTB: switchtec: Add link event notifier callback")
>> Assisted-by: Codex:deepseek-v4-flash
>> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
>> ---
>>  drivers/pci/switch/switchtec.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
>> index 5711aaa5df11..f339ea54aa38 100644
>> --- a/drivers/pci/switch/switchtec.c
>> +++ b/drivers/pci/switch/switchtec.c
>> @@ -1319,6 +1319,8 @@ static void stdev_kill(struct switchtec_dev *stdev)
>>  	pci_clear_master(stdev->pdev);
>>  
>>  	cancel_delayed_work_sync(&stdev->mrpc_timeout);
>> +	cancel_work_sync(&stdev->mrpc_work);
>> +	cancel_work_sync(&stdev->link_event_work);
> 
> I'm wondering if these should come before the mrpc_timeout sync.
> Otherwise, hypothetically, new work could be added and another timeout
> could be in progress.
> 
yes,
  	
+	cancel_work_sync(&stdev->mrpc_work);
+	cancel_work_sync(&stdev->link_event_work);
	cancel_delayed_work_sync(&stdev->mrpc_timeout);

> Also, I'm not sure, but seems like the interrupt should be disabled
> before this as well?
I looked it up and it appears that pci_clear_master cannot disable
interrupt enabling. Should I use devm_free_irq?

Thanks！
Pei.
> 
> Thanks!
> 
> Logan

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

[Logan Gunthorpe]

On 2026-08-04 19:28, Pei Xiao wrote:
>> I'm wondering if these should come before the mrpc_timeout sync.
>> Otherwise, hypothetically, new work could be added and another timeout
>> could be in progress.
>>
> yes,
>   	
> +	cancel_work_sync(&stdev->mrpc_work);
> +	cancel_work_sync(&stdev->link_event_work);
> 	cancel_delayed_work_sync(&stdev->mrpc_timeout);
> 
>> Also, I'm not sure, but seems like the interrupt should be disabled
>> before this as well?
> I looked it up and it appears that pci_clear_master cannot disable
> interrupt enabling. Should I use devm_free_irq?

Yes, I believe a couple calls to devm_free_irq() is what will be needed
here.

Logan