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

Pei Xiao <[email protected]>
Newsgroups org.kernel.vger.linux-pci,org.kernel.vger.linux-kernel,org.kernel.vger.stable
Message-ID <[email protected]>

在 2026/8/6 23:32, Logan Gunthorpe 写道:
> 
> 
> 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.
Hi Maintainer,
  I apologize for my delay, and I have already sent the third version.

https://lore.kernel.org/lkml/4cebfe41ee3985bc4f38beb42d44147a34637971.1786944920.git.xiaopei01@kylinos.cn/

Pei.
Thanks.
> 
> Other than that minor point:
> 
> Reviewed-by: Logan Gunthorpe <[email protected]>
> 
> Thanks!
> 
> Logan
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.