[PATCH] PCI: switchtec: Fix use-after-free in mrpc_timeout_work

Pei Xiao posted 1 patch 1 month, 3 weeks ago
drivers/pci/switch/switchtec.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] PCI: switchtec: Fix use-after-free in mrpc_timeout_work
Posted by Pei Xiao 1 month, 3 weeks ago
stdev_kill() cancels the works before clearing stdev->alive under
mrpc_mutex. Every path that arms mrpc_timeout holds that mutex, so a
write that already passed the alive check can still schedule
mrpc_timeout after cancel_delayed_work_sync() returns:

CPU0                                      CPU1

                                          | switchtec_dev_write()
                                          | lock_mutex_and_test_alive()
cancel_delayed_work_sync(mrpc_timeout)    |
                                          | mrpc_cmd_submit()
                                          | schedule_delayed_work(mrpc_timeout)
                                          | mutex_unlock(&stdev->mrpc_mutex)
scoped_guard(mutex) { alive = false }     |
put_device() -> kfree(stdev)              |
                                          | mrpc_timeout_work

Fix it by clearing alive under mrpc_mutex before canceling the works, so no
new mrpc_timeout can be armed once alive is cleared.

Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
Cc: stable@vger.kernel.org
Assisted-by: Codex:deepseek-v4-flash
Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
---
 drivers/pci/switch/switchtec.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
index 235ca1877b6c..9127841b2bf8 100644
--- a/drivers/pci/switch/switchtec.c
+++ b/drivers/pci/switch/switchtec.c
@@ -1323,10 +1323,6 @@ static void stdev_kill(struct switchtec_dev *stdev)
 	if (stdev->dma_mrpc_irq >= 0)
 		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 */
 	scoped_guard (mutex, &stdev->mrpc_mutex) {
 		stdev->alive = false;
@@ -1341,6 +1337,10 @@ static void stdev_kill(struct switchtec_dev *stdev)
 
 	}
 
+	cancel_work_sync(&stdev->mrpc_work);
+	cancel_work_sync(&stdev->link_event_work);
+	cancel_delayed_work_sync(&stdev->mrpc_timeout);
+
 	/* Wake up any users waiting on event_wq */
 	wake_up_interruptible(&stdev->event_wq);
 }
-- 
2.25.1
Re: [PATCH] PCI: switchtec: Fix use-after-free in mrpc_timeout_work
Posted by Manivannan Sadhasivam 3 weeks ago
On Thu, Aug 06, 2026 at 07:55:15PM +0800, Pei Xiao wrote:
> stdev_kill() cancels the works before clearing stdev->alive under
> mrpc_mutex. Every path that arms mrpc_timeout holds that mutex, so a
> write that already passed the alive check can still schedule
> mrpc_timeout after cancel_delayed_work_sync() returns:
> 
> CPU0                                      CPU1
> 
>                                           | switchtec_dev_write()
>                                           | lock_mutex_and_test_alive()
> cancel_delayed_work_sync(mrpc_timeout)    |
>                                           | mrpc_cmd_submit()
>                                           | schedule_delayed_work(mrpc_timeout)
>                                           | mutex_unlock(&stdev->mrpc_mutex)
> scoped_guard(mutex) { alive = false }     |
> put_device() -> kfree(stdev)              |
>                                           | mrpc_timeout_work
> 
> Fix it by clearing alive under mrpc_mutex before canceling the works, so no
> new mrpc_timeout can be armed once alive is cleared.
> 
> Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver")
> Cc: stable@vger.kernel.org
> Assisted-by: Codex:deepseek-v4-flash
> Signed-off-by: Pei Xiao <xiaopei01@kylinos.cn>
> ---
>  drivers/pci/switch/switchtec.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c
> index 235ca1877b6c..9127841b2bf8 100644
> --- a/drivers/pci/switch/switchtec.c
> +++ b/drivers/pci/switch/switchtec.c
> @@ -1323,10 +1323,6 @@ static void stdev_kill(struct switchtec_dev *stdev)
>  	if (stdev->dma_mrpc_irq >= 0)
>  		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);
> -

This part of the code never existed in upstream. Perhaps you are referring to
some downstream tree?

- Mani

-- 
மணிவண்ணன் சதாசிவம்