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

[Bjorn Helgaas <helgaas@kernel.org>]

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.

> ---
> 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
>