* [PATCH 1/9] PCI: cadence: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 16:18 ` [PATCH 2/9] PCI: dwc: " Hans Zhang
` (9 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Calling pci_stop_root_bus() and pci_remove_root_bus() without holding
the PCI rescan lock may race with concurrent rescan or hotplug
operations triggered via sysfs, leading to use-after-free or system
crashes.
Add pci_lock_rescan_remove() before stopping/removing the root bus and
pci_unlock_rescan_remove() after in cdns_pcie_host_disable().
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/cadence/pcie-cadence-host.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c
index 0bc9e6e90e0e..87fd8afa301b 100644
--- a/drivers/pci/controller/cadence/pcie-cadence-host.c
+++ b/drivers/pci/controller/cadence/pcie-cadence-host.c
@@ -365,8 +365,10 @@ void cdns_pcie_host_disable(struct cdns_pcie_rc *rc)
struct pci_host_bridge *bridge;
bridge = pci_host_bridge_from_priv(rc);
+ pci_lock_rescan_remove();
pci_stop_root_bus(bridge->bus);
pci_remove_root_bus(bridge->bus);
+ pci_unlock_rescan_remove();
cdns_pcie_host_deinit(rc);
cdns_pcie_host_link_disable(rc);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 2/9] PCI: dwc: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
2026-05-21 16:18 ` [PATCH 1/9] PCI: cadence: Protect root bus removal with rescan lock Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 16:18 ` [PATCH 3/9] PCI: altera: " Hans Zhang
` (8 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Add pci_lock_rescan_remove() / pci_unlock_rescan_remove() around
pci_stop_root_bus() and pci_remove_root_bus() in dw_pcie_host_deinit()
to prevent race conditions with concurrent PCI rescan or hotplug.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/dwc/pcie-designware-host.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index c9517a348836..f856ac2fb15d 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -703,8 +703,10 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
dwc_pcie_debugfs_deinit(pci);
+ pci_lock_rescan_remove();
pci_stop_root_bus(pp->bridge->bus);
pci_remove_root_bus(pp->bridge->bus);
+ pci_unlock_rescan_remove();
dw_pcie_stop_link(pci);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 3/9] PCI: altera: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
2026-05-21 16:18 ` [PATCH 1/9] PCI: cadence: Protect root bus removal with rescan lock Hans Zhang
2026-05-21 16:18 ` [PATCH 2/9] PCI: dwc: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 18:51 ` sashiko-bot
2026-05-21 16:18 ` [PATCH 4/9] PCI: brcmstb: " Hans Zhang
` (7 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
In altera_pcie_remove(), wrap pci_stop_root_bus() and
pci_remove_root_bus() with pci_lock_rescan_remove() /
pci_unlock_rescan_remove() to avoid potential races with
sysfs-triggered rescans.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-altera.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
index 3dbb7adc421c..7e1db267ae34 100644
--- a/drivers/pci/controller/pcie-altera.c
+++ b/drivers/pci/controller/pcie-altera.c
@@ -1045,8 +1045,10 @@ static void altera_pcie_remove(struct platform_device *pdev)
struct altera_pcie *pcie = platform_get_drvdata(pdev);
struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+ pci_lock_rescan_remove();
pci_stop_root_bus(bridge->bus);
pci_remove_root_bus(bridge->bus);
+ pci_unlock_rescan_remove();
altera_pcie_irq_teardown(pcie);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 3/9] PCI: altera: Protect root bus removal with rescan lock
2026-05-21 16:18 ` [PATCH 3/9] PCI: altera: " Hans Zhang
@ 2026-05-21 18:51 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-05-21 18:51 UTC (permalink / raw)
To: Hans Zhang; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing cleanup of chained IRQ handler and IRQ domain on probe failure paths in `altera_pcie_probe()`.
- [Medium] The driver does not disable hardware interrupt generation during device removal.
- [Medium] The driver erroneously calls `irq_dispose_mapping()` on the parent platform IRQ in `altera_pcie_irq_teardown()`.
--
commit c918986379b1a0013bed56503edcc992be2805fa
Author: Hans Zhang <18255117159@163.com>
PCI: altera: Protect root bus removal with rescan lock
This commit wraps pci_stop_root_bus() and pci_remove_root_bus() with
the rescan locks to avoid potential races with sysfs-triggered rescans
during device removal.
> diff --git a/drivers/pci/controller/pcie-altera.c b/drivers/pci/controller/pcie-altera.c
> index 3dbb7adc421c4..7e1db267ae342 100644
> --- a/drivers/pci/controller/pcie-altera.c
> +++ b/drivers/pci/controller/pcie-altera.c
> @@ -1045,8 +1045,10 @@ static void altera_pcie_remove(struct platform_device *pdev)
> struct altera_pcie *pcie = platform_get_drvdata(pdev);
> struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
[Severity: Medium]
This isn't a bug introduced by this patch, but does the driver need to
disable hardware interrupt generation during device removal?
The driver enables hardware interrupts by writing to device registers during
probe, but tears down the driver here without masking or disabling those
hardware interrupts.
Even though the chained handler is unregistered below, if the device still
asserts the interrupt line, could the CPU receive an interrupt with no
registered handler and cause the spurious interrupt detector to permanently
disable the IRQ line?
>
> + pci_lock_rescan_remove();
> pci_stop_root_bus(bridge->bus);
> pci_remove_root_bus(bridge->bus);
> + pci_unlock_rescan_remove();
> altera_pcie_irq_teardown(pcie);
[Severity: Medium]
This is a pre-existing issue, but does altera_pcie_irq_teardown()
incorrectly dispose of the platform IRQ mapping?
The teardown function executes irq_dispose_mapping(pcie->irq). However,
pcie->irq is retrieved via platform_get_irq(), meaning the mapping was
created and is owned by the platform subsystem.
Device drivers usually shouldn't destroy IRQ mappings they did not
explicitly allocate. Could disposing of this mapping break the lifecycle
management of the platform IRQ if the driver is rebound?
[Severity: High]
This is also a pre-existing issue not introduced here, but I noticed a
potential use-after-free in the probe error paths.
The probe function successfully registers a chained IRQ handler via
irq_set_chained_handler_and_data(pcie->irq, pcie->pcie_data->ops->rp_isr,
pcie). If a subsequent step like altera_pcie_init_irq_domain() or
pci_host_probe() fails, the function returns an error without unregistering
the chained handler.
Since the pcie structure is managed by devres, it is freed upon probe
failure. If the hardware asserts the interrupt line, could the core IRQ code
invoke the chained handler and dereference the freed pcie pointer?
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=3
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 4/9] PCI: brcmstb: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (2 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 3/9] PCI: altera: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 19:19 ` sashiko-bot
2026-05-21 16:18 ` [PATCH 5/9] PCI: iproc: " Hans Zhang
` (6 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Add missing PCI rescan lock protection in brcm_pcie_remove() before
removing the root bus.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-brcmstb.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
index 714bcab97b60..7ca27f0756c9 100644
--- a/drivers/pci/controller/pcie-brcmstb.c
+++ b/drivers/pci/controller/pcie-brcmstb.c
@@ -1894,8 +1894,10 @@ static void brcm_pcie_remove(struct platform_device *pdev)
struct brcm_pcie *pcie = platform_get_drvdata(pdev);
struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
+ pci_lock_rescan_remove();
pci_stop_root_bus(bridge->bus);
pci_remove_root_bus(bridge->bus);
+ pci_unlock_rescan_remove();
if (pcie->cfg->has_err_report)
brcm_unregister_die_notifiers(pcie);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 4/9] PCI: brcmstb: Protect root bus removal with rescan lock
2026-05-21 16:18 ` [PATCH 4/9] PCI: brcmstb: " Hans Zhang
@ 2026-05-21 19:19 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-05-21 19:19 UTC (permalink / raw)
To: Hans Zhang; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Calling `brcm_pcie_remove()` on the `pci_host_probe()` error path triggers a Use-After-Free (UAF) or NULL pointer dereference.
- [High] The `pcie->bridge_lock` is used before it is initialized during device probe.
--
commit baacf34014bfc48de11be2860d0ae507bf20e383
Author: Hans Zhang <18255117159@163.com>
PCI: brcmstb: Protect root bus removal with rescan lock
Add missing PCI rescan lock protection in brcm_pcie_remove() before
removing the root bus.
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 714bcab97b603..7ca27f0756c9b 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -1894,8 +1894,10 @@ static void brcm_pcie_remove(struct platform_device *pdev)
> struct brcm_pcie *pcie = platform_get_drvdata(pdev);
> struct pci_host_bridge *bridge = pci_host_bridge_from_priv(pcie);
>
> + pci_lock_rescan_remove();
> pci_stop_root_bus(bridge->bus);
[Severity: Critical]
This is a pre-existing issue, but can calling brcm_pcie_remove() on the
pci_host_probe() error path trigger a use-after-free or null pointer
dereference here?
In brcm_pcie_probe(), if pci_host_probe() fails, the error path directly
calls brcm_pcie_remove(). If pci_host_probe() failed early because
pci_alloc_bus() returned null, bridge->bus remains null. If it failed later
during pci_register_host_bridge(), the allocated bus is freed via kfree(),
but bridge->bus is never cleared, leaving it as a dangling pointer.
When brcm_pcie_remove() is invoked, it unconditionally calls
pci_stop_root_bus(bridge->bus). Passing a null or dangling pointer here
causes a kernel crash on the probe error path.
> pci_remove_root_bus(bridge->bus);
> + pci_unlock_rescan_remove();
> if (pcie->cfg->has_err_report)
> brcm_unregister_die_notifiers(pcie);
[Severity: High]
This is a pre-existing issue, but is pcie->bridge_lock used before it is
initialized during device probe?
During brcm_pcie_probe(), the function brcm_pcie_bridge_sw_init_set() is
called early in the initialization sequence. For devices where
pcie->cfg->has_err_report is true, this function acquires pcie->bridge_lock
via spin_lock_irqsave().
However, spin_lock_init(&pcie->bridge_lock) is not called until much later
in the probe sequence, after pci_host_probe() finishes. While the memory is
zero-initialized, unconditionally attempting to acquire an uninitialized
spinlock is structurally unsafe and can trigger lockdep warnings or crashes.
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=4
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 5/9] PCI: iproc: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (3 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 4/9] PCI: brcmstb: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 16:18 ` [PATCH 6/9] PCI: mediatek: " Hans Zhang
` (5 subsequent siblings)
10 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Hold pci_rescan_remove_global_lock during root bus stop and
removal in iproc_pcie_remove().
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-iproc.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-iproc.c b/drivers/pci/controller/pcie-iproc.c
index ccf71993ea35..c8f0a87cf28a 100644
--- a/drivers/pci/controller/pcie-iproc.c
+++ b/drivers/pci/controller/pcie-iproc.c
@@ -1529,8 +1529,10 @@ void iproc_pcie_remove(struct iproc_pcie *pcie)
{
struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
+ pci_lock_rescan_remove();
pci_stop_root_bus(host->bus);
pci_remove_root_bus(host->bus);
+ pci_unlock_rescan_remove();
iproc_pcie_msi_disable(pcie);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* [PATCH 6/9] PCI: mediatek: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (4 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 5/9] PCI: iproc: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 20:11 ` sashiko-bot
2026-05-21 16:18 ` [PATCH 7/9] PCI: rockchip: " Hans Zhang
` (4 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Secure mtk_pcie_remove() by adding pci_lock_rescan_remove() /
pci_unlock_rescan_remove() around pci_stop_root_bus() and
pci_remove_root_bus().
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-mediatek.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
index 75722524fe74..2fedb6d2939d 100644
--- a/drivers/pci/controller/pcie-mediatek.c
+++ b/drivers/pci/controller/pcie-mediatek.c
@@ -1172,8 +1172,10 @@ static void mtk_pcie_remove(struct platform_device *pdev)
struct mtk_pcie *pcie = platform_get_drvdata(pdev);
struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
+ pci_lock_rescan_remove();
pci_stop_root_bus(host->bus);
pci_remove_root_bus(host->bus);
+ pci_unlock_rescan_remove();
mtk_pcie_free_resources(pcie);
mtk_pcie_irq_teardown(pcie);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 6/9] PCI: mediatek: Protect root bus removal with rescan lock
2026-05-21 16:18 ` [PATCH 6/9] PCI: mediatek: " Hans Zhang
@ 2026-05-21 20:11 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-05-21 20:11 UTC (permalink / raw)
To: Hans Zhang; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unbalanced clock disable and PM runtime disable when PCIe links are down.
--
commit 92b7c7047c2235e7192b983ddc544d37ac83ba04
Author: Hans Zhang <18255117159@163.com>
PCI: mediatek: Protect root bus removal with rescan lock
Secure mtk_pcie_remove() by adding pci_lock_rescan_remove() /
pci_unlock_rescan_remove() around pci_stop_root_bus() and
pci_remove_root_bus().
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 75722524fe744..2fedb6d2939de 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -1172,8 +1172,10 @@ static void mtk_pcie_remove(struct platform_device *pdev)
> struct mtk_pcie *pcie = platform_get_drvdata(pdev);
> struct pci_host_bridge *host = pci_host_bridge_from_priv(pcie);
>
> + pci_lock_rescan_remove();
> pci_stop_root_bus(host->bus);
> pci_remove_root_bus(host->bus);
> + pci_unlock_rescan_remove();
> mtk_pcie_free_resources(pcie);
>
> mtk_pcie_irq_teardown(pcie);
[Severity: High]
This is a pre-existing issue, but does the unconditional call to
mtk_pcie_put_resources(pcie) at the end of this function cause unbalanced
disables if all port links failed to come up?
In mtk_pcie_setup(), if all port links fail to come up (for instance, if no
PCIe endpoint is connected), list_empty(&pcie->ports) evaluates to true.
When this happens, mtk_pcie_subsys_powerdown(pcie) is called early,
disabling the clock and PM runtime. However, mtk_pcie_probe() successfully
completes and registers the empty root bus.
When the driver is later unbound or removed, mtk_pcie_remove()
unconditionally executes mtk_pcie_put_resources(pcie), without the necessary
!list_empty(&pcie->ports) guard that correctly exists in the probe error
path. This calls mtk_pcie_subsys_powerdown() a second time.
Will this double clk_disable_unprepare() trigger a clock framework WARN,
and increment the PM disable depth to 2?
If the driver is later re-probed, pm_runtime_enable() only decrements the
depth by 1, leaving the device disabled.
Can this result in pm_runtime_get_sync() silently failing to power on the
PCIe power domain, causing subsequent MMIO register accesses in
mtk_pcie_enable_port() to trigger a synchronous external abort?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=6
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 7/9] PCI: rockchip: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (5 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 6/9] PCI: mediatek: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 20:40 ` sashiko-bot
2026-05-21 16:18 ` [PATCH 8/9] PCI: vmd: " Hans Zhang
` (3 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
In rockchip_pcie_remove(), use PCI rescan lock to serialize root bus
deletion against concurrent bus rescan operations.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/pcie-rockchip-host.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..d203c4876d30 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -1012,8 +1012,10 @@ static void rockchip_pcie_remove(struct platform_device *pdev)
struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
struct pci_host_bridge *bridge = pci_host_bridge_from_priv(rockchip);
+ pci_lock_rescan_remove();
pci_stop_root_bus(bridge->bus);
pci_remove_root_bus(bridge->bus);
+ pci_unlock_rescan_remove();
irq_domain_remove(rockchip->irq_domain);
rockchip_pcie_deinit_phys(rockchip);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 7/9] PCI: rockchip: Protect root bus removal with rescan lock
2026-05-21 16:18 ` [PATCH 7/9] PCI: rockchip: " Hans Zhang
@ 2026-05-21 20:40 ` sashiko-bot
0 siblings, 0 replies; 22+ messages in thread
From: sashiko-bot @ 2026-05-21 20:40 UTC (permalink / raw)
To: Hans Zhang; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The chained IRQ handler for the 'legacy' interrupt is never cleared during driver removal or probe failure, leaving a dangling pointer that can cause a use-after-free crash.
- [High] Devm-managed interrupt handlers are not unregistered before hardware clocks and regulators are disabled, risking a synchronous abort if an IRQ triggers during or after teardown.
--
commit d2ff4e68d35e6bd8f1e9996d0dafb48fdb147cab
Author: Hans Zhang <18255117159@163.com>
PCI: rockchip: Protect root bus removal with rescan lock
This patch modifies the rockchip PCIe host driver to wrap the root bus
removal operations with the PCI rescan/remove lock. This serializes bus
deletion against concurrent bus rescan operations.
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -1012,8 +1012,10 @@ static void rockchip_pcie_remove(struct platform_device *pdev)
> struct rockchip_pcie *rockchip = dev_get_drvdata(dev);
> struct pci_host_bridge *bridge = pci_host_bridge_from_priv(rockchip);
>
> + pci_lock_rescan_remove();
> pci_stop_root_bus(bridge->bus);
> pci_remove_root_bus(bridge->bus);
> + pci_unlock_rescan_remove();
> irq_domain_remove(rockchip->irq_domain);
>
> rockchip_pcie_deinit_phys(rockchip);
[Severity: High]
This isn't a bug introduced by this patch, but I have a question about the
interrupt teardown sequence that follows this code.
The sys and client interrupts are registered using devm_request_irq() with
the IRQF_SHARED flag. Because they are devm-managed, they remain active
until the devres teardown occurs, which happens strictly after
rockchip_pcie_remove() returns.
Later in this function, hardware clocks and regulators are disabled:
rockchip_pcie_disable_clocks(rockchip);
if (!IS_ERR(rockchip->vpcie12v))
regulator_disable(rockchip->vpcie12v);
...
If a shared or spurious interrupt fires after the hardware clocks and
regulators are disabled, will the handlers attempt to read PCIe MMIO
registers? Could reading from unclocked or unpowered peripheral registers
trigger a synchronous external abort?
Should devm_free_irq() be called for these interrupts before the clocks
are disabled?
[Severity: Critical]
This is also a pre-existing issue, but I noticed the chained IRQ handler
for the legacy interrupt is never cleared during driver removal.
In rockchip_pcie_setup_irq(), a chained IRQ handler is set for the legacy
interrupt:
irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
rockchip);
Unlike standard interrupt requests via devm_request_irq(), chained handlers
are not managed by devres and must be explicitly cleared.
Since the rockchip structure is allocated using devm_pci_alloc_host_bridge(),
it is freed by devres when the device is unbound. If a legacy interrupt
fires after the device memory is freed, could the IRQ core invoke
rockchip_pcie_intx_handler() and dereference the freed rockchip structure?
Should irq_set_chained_handler_and_data(irq, NULL, NULL) be called at the
end of rockchip_pcie_remove() to prevent a use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=7
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (6 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 7/9] PCI: rockchip: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-05-21 21:20 ` sashiko-bot
2026-05-21 16:18 ` [PATCH 9/9] PCI: plda: " Hans Zhang
` (2 subsequent siblings)
10 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Hold pci_rescan_remove_global_lock while stopping and removing the VMD
root bus in vmd_remove() to prevent races with PCI rescan operations.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/vmd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d4ae250d4bc6..9565567026cf 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
{
struct vmd_dev *vmd = pci_get_drvdata(dev);
+ pci_lock_rescan_remove();
pci_stop_root_bus(vmd->bus);
sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
pci_remove_root_bus(vmd->bus);
+ pci_unlock_rescan_remove();
vmd_cleanup_srcu(vmd);
vmd_detach_resources(vmd);
vmd_remove_irq_domain(vmd);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-05-21 16:18 ` [PATCH 8/9] PCI: vmd: " Hans Zhang
@ 2026-05-21 21:20 ` sashiko-bot
2026-06-10 17:28 ` Manivannan Sadhasivam
0 siblings, 1 reply; 22+ messages in thread
From: sashiko-bot @ 2026-05-21 21:20 UTC (permalink / raw)
To: Hans Zhang; +Cc: linux-pci
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Acquiring `pci_rescan_remove_lock` in `vmd_remove()` introduces self-deadlocks and AB-BA lock inversions.
Pre-existing issues:
- [High] SRCU structures are destroyed in `vmd_remove()` while their corresponding interrupt handlers are still registered and active, leading to a Use-After-Free in interrupt context.
--
commit 8d5d06b67e6b02d2cc99f2151e90ba61c0d16662
Author: Hans Zhang <18255117159@163.com>
PCI: vmd: Protect root bus removal with rescan lock
This commit acquires the pci_rescan_remove_global_lock while stopping
and removing the VMD root bus in vmd_remove(). This is intended to
prevent races with concurrent PCI rescan operations.
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..9565567026cf0 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
> {
> struct vmd_dev *vmd = pci_get_drvdata(dev);
>
> + pci_lock_rescan_remove();
[Severity: High]
Can acquiring pci_rescan_remove_lock here cause self-deadlocks or AB-BA lock
inversions?
When a VMD device is removed via PCI hotplug or sysfs remove node, the PCI
core calls pci_stop_and_remove_bus_device_locked(), which acquires
pci_rescan_remove_lock and eventually triggers vmd_remove(). Because
pci_rescan_remove_lock is a non-recursive mutex, won't attempting to acquire
it again immediately self-deadlock?
Also, when unbound via sysfs, the core holds device_lock and calls
vmd_remove(), which then waits for pci_rescan_remove_lock. Since normal PCI
rescans acquire pci_rescan_remove_lock first, then attempt to acquire
device_lock, does this create an AB-BA lock inversion?
> pci_stop_root_bus(vmd->bus);
> sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
> pci_remove_root_bus(vmd->bus);
> + pci_unlock_rescan_remove();
> vmd_cleanup_srcu(vmd);
[Severity: High]
This is a pre-existing issue, but does this code cause a use-after-free in
interrupt context?
The driver registers its interrupts using devm_request_irq(), which means
devres runs after vmd_remove() returns, leaving the IRQ handlers active
throughout device removal.
If an interrupt fires during this window, won't the active vmd_irq() handler
call srcu_read_lock() on the destroyed SRCU structure and result in a
use-after-free of per-CPU memory?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260521161822.132996-1-18255117159@163.com?part=8
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-05-21 21:20 ` sashiko-bot
@ 2026-06-10 17:28 ` Manivannan Sadhasivam
2026-06-11 4:42 ` Hans Zhang
0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-06-10 17:28 UTC (permalink / raw)
To: sashiko-reviews; +Cc: Hans Zhang, linux-pci
On Thu, May 21, 2026 at 09:20:24PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Acquiring `pci_rescan_remove_lock` in `vmd_remove()` introduces self-deadlocks and AB-BA lock inversions.
>
> Pre-existing issues:
> - [High] SRCU structures are destroyed in `vmd_remove()` while their corresponding interrupt handlers are still registered and active, leading to a Use-After-Free in interrupt context.
> --
>
> commit 8d5d06b67e6b02d2cc99f2151e90ba61c0d16662
> Author: Hans Zhang <18255117159@163.com>
>
> PCI: vmd: Protect root bus removal with rescan lock
>
> This commit acquires the pci_rescan_remove_global_lock while stopping
> and removing the VMD root bus in vmd_remove(). This is intended to
> prevent races with concurrent PCI rescan operations.
>
> > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > index d4ae250d4bc6d..9565567026cf0 100644
> > --- a/drivers/pci/controller/vmd.c
> > +++ b/drivers/pci/controller/vmd.c
> > @@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
> > {
> > struct vmd_dev *vmd = pci_get_drvdata(dev);
> >
> > + pci_lock_rescan_remove();
>
> [Severity: High]
> Can acquiring pci_rescan_remove_lock here cause self-deadlocks or AB-BA lock
> inversions?
>
> When a VMD device is removed via PCI hotplug or sysfs remove node, the PCI
> core calls pci_stop_and_remove_bus_device_locked(), which acquires
> pci_rescan_remove_lock and eventually triggers vmd_remove(). Because
> pci_rescan_remove_lock is a non-recursive mutex, won't attempting to acquire
> it again immediately self-deadlock?
>
> Also, when unbound via sysfs, the core holds device_lock and calls
> vmd_remove(), which then waits for pci_rescan_remove_lock. Since normal PCI
> rescans acquire pci_rescan_remove_lock first, then attempt to acquire
> device_lock, does this create an AB-BA lock inversion?
>
This is a valid concern. Since VMD is registered as a PCI driver, removing the
VMD device through sysfs will trigger vmd_remove() callback and it will result
in deadlock. The AB-BA lock inversion is also valid.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-06-10 17:28 ` Manivannan Sadhasivam
@ 2026-06-11 4:42 ` Hans Zhang
2026-06-11 6:30 ` Manivannan Sadhasivam
0 siblings, 1 reply; 22+ messages in thread
From: Hans Zhang @ 2026-06-11 4:42 UTC (permalink / raw)
To: Manivannan Sadhasivam, sashiko-reviews; +Cc: linux-pci
On 6/11/26 01:28, Manivannan Sadhasivam wrote:
> On Thu, May 21, 2026 at 09:20:24PM +0000, sashiko-bot@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> New issues:
>> - [High] Acquiring `pci_rescan_remove_lock` in `vmd_remove()` introduces self-deadlocks and AB-BA lock inversions.
>>
>> Pre-existing issues:
>> - [High] SRCU structures are destroyed in `vmd_remove()` while their corresponding interrupt handlers are still registered and active, leading to a Use-After-Free in interrupt context.
>> --
>>
>> commit 8d5d06b67e6b02d2cc99f2151e90ba61c0d16662
>> Author: Hans Zhang <18255117159@163.com>
>>
>> PCI: vmd: Protect root bus removal with rescan lock
>>
>> This commit acquires the pci_rescan_remove_global_lock while stopping
>> and removing the VMD root bus in vmd_remove(). This is intended to
>> prevent races with concurrent PCI rescan operations.
>>
>>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
>>> index d4ae250d4bc6d..9565567026cf0 100644
>>> --- a/drivers/pci/controller/vmd.c
>>> +++ b/drivers/pci/controller/vmd.c
>>> @@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
>>> {
>>> struct vmd_dev *vmd = pci_get_drvdata(dev);
>>>
>>> + pci_lock_rescan_remove();
>>
>> [Severity: High]
>> Can acquiring pci_rescan_remove_lock here cause self-deadlocks or AB-BA lock
>> inversions?
>>
>> When a VMD device is removed via PCI hotplug or sysfs remove node, the PCI
>> core calls pci_stop_and_remove_bus_device_locked(), which acquires
>> pci_rescan_remove_lock and eventually triggers vmd_remove(). Because
>> pci_rescan_remove_lock is a non-recursive mutex, won't attempting to acquire
>> it again immediately self-deadlock?
>>
>> Also, when unbound via sysfs, the core holds device_lock and calls
>> vmd_remove(), which then waits for pci_rescan_remove_lock. Since normal PCI
>> rescans acquire pci_rescan_remove_lock first, then attempt to acquire
>> device_lock, does this create an AB-BA lock inversion?
>>
>
> This is a valid concern. Since VMD is registered as a PCI driver, removing the
> VMD device through sysfs will trigger vmd_remove() callback and it will result
> in deadlock. The AB-BA lock inversion is also valid.
>
Hi Mani,
Thank you for your reply. I checked it and found that this is indeed the
registered PCI driver. This patch will be dropped from this series. I
wonder if you have any further comments or opinions regarding the
remaining patches. Do I need to send the next version?
Best regards,
Hans
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-06-11 4:42 ` Hans Zhang
@ 2026-06-11 6:30 ` Manivannan Sadhasivam
2026-06-11 14:00 ` Hans Zhang
0 siblings, 1 reply; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-06-11 6:30 UTC (permalink / raw)
To: Hans Zhang; +Cc: sashiko-reviews, linux-pci
On Thu, Jun 11, 2026 at 12:42:52PM +0800, Hans Zhang wrote:
>
>
> On 6/11/26 01:28, Manivannan Sadhasivam wrote:
> > On Thu, May 21, 2026 at 09:20:24PM +0000, sashiko-bot@kernel.org wrote:
> > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > >
> > > New issues:
> > > - [High] Acquiring `pci_rescan_remove_lock` in `vmd_remove()` introduces self-deadlocks and AB-BA lock inversions.
> > >
> > > Pre-existing issues:
> > > - [High] SRCU structures are destroyed in `vmd_remove()` while their corresponding interrupt handlers are still registered and active, leading to a Use-After-Free in interrupt context.
> > > --
> > >
> > > commit 8d5d06b67e6b02d2cc99f2151e90ba61c0d16662
> > > Author: Hans Zhang <18255117159@163.com>
> > >
> > > PCI: vmd: Protect root bus removal with rescan lock
> > >
> > > This commit acquires the pci_rescan_remove_global_lock while stopping
> > > and removing the VMD root bus in vmd_remove(). This is intended to
> > > prevent races with concurrent PCI rescan operations.
> > >
> > > > diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> > > > index d4ae250d4bc6d..9565567026cf0 100644
> > > > --- a/drivers/pci/controller/vmd.c
> > > > +++ b/drivers/pci/controller/vmd.c
> > > > @@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
> > > > {
> > > > struct vmd_dev *vmd = pci_get_drvdata(dev);
> > > > + pci_lock_rescan_remove();
> > >
> > > [Severity: High]
> > > Can acquiring pci_rescan_remove_lock here cause self-deadlocks or AB-BA lock
> > > inversions?
> > >
> > > When a VMD device is removed via PCI hotplug or sysfs remove node, the PCI
> > > core calls pci_stop_and_remove_bus_device_locked(), which acquires
> > > pci_rescan_remove_lock and eventually triggers vmd_remove(). Because
> > > pci_rescan_remove_lock is a non-recursive mutex, won't attempting to acquire
> > > it again immediately self-deadlock?
> > >
> > > Also, when unbound via sysfs, the core holds device_lock and calls
> > > vmd_remove(), which then waits for pci_rescan_remove_lock. Since normal PCI
> > > rescans acquire pci_rescan_remove_lock first, then attempt to acquire
> > > device_lock, does this create an AB-BA lock inversion?
> > >
> >
> > This is a valid concern. Since VMD is registered as a PCI driver, removing the
> > VMD device through sysfs will trigger vmd_remove() callback and it will result
> > in deadlock. The AB-BA lock inversion is also valid.
> >
>
> Hi Mani,
>
> Thank you for your reply. I checked it and found that this is indeed the
> registered PCI driver. This patch will be dropped from this series. I wonder
> if you have any further comments or opinions regarding the remaining
> patches. Do I need to send the next version?
>
I will skip this patch and apply the rest.
- Mani
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 8/9] PCI: vmd: Protect root bus removal with rescan lock
2026-06-11 6:30 ` Manivannan Sadhasivam
@ 2026-06-11 14:00 ` Hans Zhang
0 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-06-11 14:00 UTC (permalink / raw)
To: Manivannan Sadhasivam; +Cc: sashiko-reviews, linux-pci
On 6/11/26 14:30, Manivannan Sadhasivam wrote:
> On Thu, Jun 11, 2026 at 12:42:52PM +0800, Hans Zhang wrote:
>>
>>
>> On 6/11/26 01:28, Manivannan Sadhasivam wrote:
>>> On Thu, May 21, 2026 at 09:20:24PM +0000, sashiko-bot@kernel.org wrote:
>>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>>>
>>>> New issues:
>>>> - [High] Acquiring `pci_rescan_remove_lock` in `vmd_remove()` introduces self-deadlocks and AB-BA lock inversions.
>>>>
>>>> Pre-existing issues:
>>>> - [High] SRCU structures are destroyed in `vmd_remove()` while their corresponding interrupt handlers are still registered and active, leading to a Use-After-Free in interrupt context.
>>>> --
>>>>
>>>> commit 8d5d06b67e6b02d2cc99f2151e90ba61c0d16662
>>>> Author: Hans Zhang <18255117159@163.com>
>>>>
>>>> PCI: vmd: Protect root bus removal with rescan lock
>>>>
>>>> This commit acquires the pci_rescan_remove_global_lock while stopping
>>>> and removing the VMD root bus in vmd_remove(). This is intended to
>>>> prevent races with concurrent PCI rescan operations.
>>>>
>>>>> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
>>>>> index d4ae250d4bc6d..9565567026cf0 100644
>>>>> --- a/drivers/pci/controller/vmd.c
>>>>> +++ b/drivers/pci/controller/vmd.c
>>>>> @@ -1056,9 +1056,11 @@ static void vmd_remove(struct pci_dev *dev)
>>>>> {
>>>>> struct vmd_dev *vmd = pci_get_drvdata(dev);
>>>>> + pci_lock_rescan_remove();
>>>>
>>>> [Severity: High]
>>>> Can acquiring pci_rescan_remove_lock here cause self-deadlocks or AB-BA lock
>>>> inversions?
>>>>
>>>> When a VMD device is removed via PCI hotplug or sysfs remove node, the PCI
>>>> core calls pci_stop_and_remove_bus_device_locked(), which acquires
>>>> pci_rescan_remove_lock and eventually triggers vmd_remove(). Because
>>>> pci_rescan_remove_lock is a non-recursive mutex, won't attempting to acquire
>>>> it again immediately self-deadlock?
>>>>
>>>> Also, when unbound via sysfs, the core holds device_lock and calls
>>>> vmd_remove(), which then waits for pci_rescan_remove_lock. Since normal PCI
>>>> rescans acquire pci_rescan_remove_lock first, then attempt to acquire
>>>> device_lock, does this create an AB-BA lock inversion?
>>>>
>>>
>>> This is a valid concern. Since VMD is registered as a PCI driver, removing the
>>> VMD device through sysfs will trigger vmd_remove() callback and it will result
>>> in deadlock. The AB-BA lock inversion is also valid.
>>>
>>
>> Hi Mani,
>>
>> Thank you for your reply. I checked it and found that this is indeed the
>> registered PCI driver. This patch will be dropped from this series. I wonder
>> if you have any further comments or opinions regarding the remaining
>> patches. Do I need to send the next version?
>>
>
> I will skip this patch and apply the rest.
>
Thank you.
> - Mani
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH 9/9] PCI: plda: Protect root bus removal with rescan lock
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (7 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 8/9] PCI: vmd: " Hans Zhang
@ 2026-05-21 16:18 ` Hans Zhang
2026-06-11 7:04 ` (subset) [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Manivannan Sadhasivam
2026-06-18 16:59 ` Bjorn Helgaas
10 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-05-21 16:18 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, mani
Cc: robh, a-garg7, linux-pci, linux-kernel, Hans Zhang
Add missing lock acquisition in plda_pcie_host_deinit() before
stopping/removing the root bus.
Signed-off-by: Hans Zhang <18255117159@163.com>
---
drivers/pci/controller/plda/pcie-plda-host.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pci/controller/plda/pcie-plda-host.c b/drivers/pci/controller/plda/pcie-plda-host.c
index 3c2f68383010..f9a34f323ad8 100644
--- a/drivers/pci/controller/plda/pcie-plda-host.c
+++ b/drivers/pci/controller/plda/pcie-plda-host.c
@@ -640,8 +640,10 @@ EXPORT_SYMBOL_GPL(plda_pcie_host_init);
void plda_pcie_host_deinit(struct plda_pcie_rp *port)
{
+ pci_lock_rescan_remove();
pci_stop_root_bus(port->bridge->bus);
pci_remove_root_bus(port->bridge->bus);
+ pci_unlock_rescan_remove();
plda_pcie_irq_domain_deinit(port);
--
2.34.1
^ permalink raw reply related [flat|nested] 22+ messages in thread* Re: (subset) [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (8 preceding siblings ...)
2026-05-21 16:18 ` [PATCH 9/9] PCI: plda: " Hans Zhang
@ 2026-06-11 7:04 ` Manivannan Sadhasivam
2026-06-18 16:59 ` Bjorn Helgaas
10 siblings, 0 replies; 22+ messages in thread
From: Manivannan Sadhasivam @ 2026-06-11 7:04 UTC (permalink / raw)
To: bhelgaas, lpieralisi, kwilczynski, Hans Zhang
Cc: robh, a-garg7, linux-pci, linux-kernel
On Fri, 22 May 2026 00:18:13 +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.
>
> [...]
Applied, thanks!
[1/9] PCI: cadence: Protect root bus removal with rescan lock
commit: 3bc5dbef170b1f715f13102fce22a5db921f893a
[2/9] PCI: dwc: Protect root bus removal with rescan lock
commit: f47cc36e63dc9b8f20c3404c701a7aed105938a2
[3/9] PCI: altera: Protect root bus removal with rescan lock
commit: e2557f2705760101e794ba32a08586b2ab35deb2
[4/9] PCI: brcmstb: Protect root bus removal with rescan lock
commit: 435c29e2af6aeaf1e115db179020ef0869cfd205
[5/9] PCI: iproc: Protect root bus removal with rescan lock
commit: 95cb96f6693ffc59a620379d74ab6f0efe65218b
[6/9] PCI: mediatek: Protect root bus removal with rescan lock
commit: abd23e96d429551f92febd7f5d4a9a63d75c5f74
[7/9] PCI: rockchip: Protect root bus removal with rescan lock
commit: f4e77bd57d9bf799bdfc125644838918128c0537
[9/9] PCI: plda: Protect root bus removal with rescan lock
commit: e94a7411ee6d3c81039e960e9c92dcee03699031
Best regards,
--
Manivannan Sadhasivam <mani@kernel.org>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal
2026-05-21 16:18 [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Hans Zhang
` (9 preceding siblings ...)
2026-06-11 7:04 ` (subset) [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal Manivannan Sadhasivam
@ 2026-06-18 16:59 ` Bjorn Helgaas
2026-06-19 15:20 ` Hans Zhang
10 siblings, 1 reply; 22+ messages in thread
From: Bjorn Helgaas @ 2026-06-18 16:59 UTC (permalink / raw)
To: Hans Zhang
Cc: bhelgaas, lpieralisi, kwilczynski, mani, robh, a-garg7, linux-pci,
linux-kernel
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
>
^ permalink raw reply [flat|nested] 22+ messages in thread* Re: [PATCH 0/9] PCI: controller: Add missing rescan lock around root bus removal
2026-06-18 16:59 ` Bjorn Helgaas
@ 2026-06-19 15:20 ` Hans Zhang
0 siblings, 0 replies; 22+ messages in thread
From: Hans Zhang @ 2026-06-19 15:20 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: bhelgaas, lpieralisi, kwilczynski, mani, robh, a-garg7, linux-pci,
linux-kernel
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
>>
^ permalink raw reply [flat|nested] 22+ messages in thread