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

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 <[email protected]>
> ---
>  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