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

[Hans Zhang]

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@xxxxxxxxxx review.
> >
> > https://lore.kernel.org/linux-pci/20260519132017.63E1BC2BCB3@xxxxxxxxxxxxxxx/
> > """
> > > +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