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

On 2026-08-05 8:12 p.m., 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 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
> Signed-off-by: Pei Xiao <[email protected]>
> ---
> 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..235ca1877b6c 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 >= 0)
> +		devm_free_irq(&stdev->pdev->dev, stdev->event_irq, 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 */
> @@ -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 = -1;
> +	stdev->dma_mrpc_irq = -1;

I think the patch as a whole is correct. But one minor nit that could
maybe clean it up slightly. pci_irq_vector() cannot return 0. So I think
it would be fine to just leave these explictily initialized to zero and
check for non-zero in the tests above.

Other than that minor point:

Reviewed-by: Logan Gunthorpe <[email protected]>

Thanks!

Logan