Re: [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal

[Hans Zhang <18255117159@163.com>]

On 6/19/26 00:59, Bjorn Helgaas wrote:
> On Fri, May 22, 2026 at 12:18:13AM +0800, Hans Zhang wrote:
>> Several PCIe host controller drivers call pci_stop_root_bus() and
>> pci_remove_root_bus() without holding the global PCI rescan lock
>> (pci_rescan_remove_global_lock). This can cause race conditions with
>> concurrent PCI rescan or hotplug operations triggered via sysfs,
>> potentially leading to use-after-free or system crashes.
>>
>> This series adds the missing lock pairs to all drivers currently
>> exhibiting this pattern. No functional change other than proper
>> synchronization.
>>
>> Each patch is independent and targets a specific controller driver.
>> Please review and apply.
> 
> Thanks for this work.  I amended the commit logs of the eight applied
> patches so they're all the same (since all the patches do exactly the
> same thing) and so they all mention the reason for the change.
> 
> There are only four callers of pci_stop_root_bus() and
> pci_remove_root_bus() that would not be able to use a trivial helper
> function that does the lock/stop/remove/unlock internally:
> 
>    zpci_bus_release
>    acpi_pci_root_remove
>    hv_pci_remove
>    vmd_remove
> 
> It would be nice to analyze those to see if they could use the same
> pattern, which could be done inside a helper function that would avoid
> the possibility of adding the same bug elsewhere.
> 

Hi Bjorn,

Thank you for reviewing the series and for the constructive suggestion
regarding the common helper function.

I fully agree that introducing a helper (e.g., pci_remove_root_bus_safe())
to encapsulate the lock/unlock logic is a cleaner and more maintainable
approach, and it would help prevent similar issues in the future.

Since we are now in the merge window, I would prefer to hold off on this
further refactoring until the next cycle to avoid introducing any
unnecessary risks or conflicts at this late stage.

Best regards,
Hans

>> ---
>> The following are the comments from the sashiko-bot@kernel.org review.
>>
>> https://lore.kernel.org/linux-pci/20260519132017.63E1BC2BCB3@smtp.kernel.org/
>> """
>>> +void cdns_pcie_hpa_host_disable(struct cdns_pcie_rc *rc)
>>> +{
>>> +	struct pci_host_bridge *bridge;
>>> +
>>> +	cdns_pcie_debugfs_deinit(&rc->pcie);
>>> +	bridge = pci_host_bridge_from_priv(rc);
>>> +	pci_stop_root_bus(bridge->bus);
>>> +	pci_remove_root_bus(bridge->bus);
>>> +}
>>
>> Could this lead to a race condition during root bus teardown?
>>
>> Calling pci_stop_root_bus() and pci_remove_root_bus() without holding the
>> PCI rescan lock allows race conditions with concurrent PCI rescan or hotplug
>> operations triggered via sysfs.
>>
>> While this pattern might have been copied from a pre-existing function like
>> cdns_pcie_host_disable(), does it need to be fixed here to prevent
>> use-after-free or system crashes?
>>
>> Should these operations be protected by acquiring the lock using
>> pci_lock_rescan_remove() and pci_unlock_rescan_remove()?
>> """
>>
>>
>> The function cdns_pcie_hpa_host_disable() mentioned in the bot's comment is from
>> a separate patch series and will be fixed independently. This series focuses only
>> on existing controller drivers in the mainline kernel.
>> ---
>>
>> Hans Zhang (9):
>>    PCI: cadence: Protect root bus removal with rescan lock
>>    PCI: dwc: Protect root bus removal with rescan lock
>>    PCI: altera: Protect root bus removal with rescan lock
>>    PCI: brcmstb: Protect root bus removal with rescan lock
>>    PCI: iproc: Protect root bus removal with rescan lock
>>    PCI: mediatek: Protect root bus removal with rescan lock
>>    PCI: rockchip: Protect root bus removal with rescan lock
>>    PCI: vmd: Protect root bus removal with rescan lock
>>    PCI: plda: Protect root bus removal with rescan lock
>>
>>   drivers/pci/controller/cadence/pcie-cadence-host.c | 2 ++
>>   drivers/pci/controller/dwc/pcie-designware-host.c  | 2 ++
>>   drivers/pci/controller/pcie-altera.c               | 2 ++
>>   drivers/pci/controller/pcie-brcmstb.c              | 2 ++
>>   drivers/pci/controller/pcie-iproc.c                | 2 ++
>>   drivers/pci/controller/pcie-mediatek.c             | 2 ++
>>   drivers/pci/controller/pcie-rockchip-host.c        | 2 ++
>>   drivers/pci/controller/plda/pcie-plda-host.c       | 2 ++
>>   drivers/pci/controller/vmd.c                       | 2 ++
>>   9 files changed, 18 insertions(+)
>>
>>
>> base-commit: 6916d5703ddf9a38f1f6c2cc793381a24ee914c6
>> -- 
>> 2.34.1
>>