[PATCH v2] PCI: keystone: fix endpoint teardown on remove

[PATCH v2] PCI: keystone: fix endpoint teardown on remove

[Guangshuo Li]

ks_pcie_probe() initializes the DesignWare PCIe endpoint with
dw_pcie_ep_init() and notifies the endpoint framework after the
endpoint registers have been initialized. However, the remove path
does not perform the corresponding endpoint teardown.

This leaves the EPC initialization state and resources allocated by
the DesignWare endpoint core active after the driver is removed.

Make dw_pcie_ep_deinit() notify the endpoint framework when endpoint
initialization has completed, so controller drivers do not need to
duplicate the notification before endpoint teardown. Keep the
notification conditional since dw_pcie_ep_deinit() is also used on
error paths before pci_epc_init_notify() has been called.

Remove the now redundant explicit notification from the STM32 endpoint
driver.

Cache the controller mode in struct keystone_pcie and use it during
removal to deinitialize the endpoint before runtime PM and the PHYs are
disabled.

Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
v2:
  - Cache the PCIe mode in struct keystone_pcie instead of looking up the
    match data again during removal.
  - Move pci_epc_deinit_notify() into dw_pcie_ep_deinit() to avoid
    duplicating the notification in controller drivers.
  - Remove the now redundant explicit deinit notification from the STM32
    endpoint driver.

 drivers/pci/controller/dwc/pci-keystone.c       | 6 ++++++
 drivers/pci/controller/dwc/pcie-designware-ep.c | 3 +++
 drivers/pci/controller/dwc/pcie-stm32-ep.c      | 1 -
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 602516239a57..59f7b2e152a2 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -121,6 +121,7 @@ struct ks_pcie_of_data {
 
 struct keystone_pcie {
 	struct dw_pcie		*pci;
+	enum dw_pcie_device_mode mode;
 	/* PCI Device ID */
 	u32			device_id;
 	int			intx_host_irqs[PCI_NUM_INTX];
@@ -1226,6 +1227,7 @@ static int ks_pcie_probe(struct platform_device *pdev)
 
 	ks_pcie->np = np;
 	ks_pcie->pci = pci;
+	ks_pcie->mode = mode;
 	ks_pcie->link = link;
 	ks_pcie->num_lanes = num_lanes;
 	ks_pcie->phy = phy;
@@ -1349,9 +1351,13 @@ static void ks_pcie_remove(struct platform_device *pdev)
 {
 	struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
 	struct device_link **link = ks_pcie->link;
+	struct dw_pcie *pci = ks_pcie->pci;
 	int num_lanes = ks_pcie->num_lanes;
 	struct device *dev = &pdev->dev;
 
+	if (ks_pcie->mode == DW_PCIE_EP_TYPE)
+		dw_pcie_ep_deinit(&pci->ep);
+
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 	ks_pcie_disable_phy(ks_pcie);
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index de8ee3db4360..b785e098f759 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -1188,6 +1188,9 @@ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
 {
 	struct pci_epc *epc = ep->epc;
 
+	if (epc->init_complete)
+		pci_epc_deinit_notify(epc);
+
 	dw_pcie_ep_cleanup(ep);
 
 	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,
diff --git a/drivers/pci/controller/dwc/pcie-stm32-ep.c b/drivers/pci/controller/dwc/pcie-stm32-ep.c
index a7988dff1045..1cb31bd76da5 100644
--- a/drivers/pci/controller/dwc/pcie-stm32-ep.c
+++ b/drivers/pci/controller/dwc/pcie-stm32-ep.c
@@ -304,7 +304,6 @@ static void stm32_pcie_remove(struct platform_device *pdev)
 
 	dw_pcie_stop_link(pci);
 
-	pci_epc_deinit_notify(ep->epc);
 	dw_pcie_ep_deinit(ep);
 
 	stm32_pcie_disable_resources(stm32_pcie);
-- 
2.43.0

Re: [PATCH v2] PCI: keystone: fix endpoint teardown on remove

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: TOCTOU race in dw_pcie_ep_deinit() causing duplicate EPC deinit
--

commit e6751d7bce3085ac260a82fa5b3a315cc5808621
Author: Guangshuo Li <lgs201920130244@gmail.com>

PCI: keystone: fix endpoint teardown on remove

This commit fixes missing endpoint teardown in the keystone PCIe driver's
remove path. It makes dw_pcie_ep_deinit() notify the endpoint framework of
deinitialization directly, avoiding duplicated notification code in
controller drivers. It also caches the controller mode in struct
keystone_pcie to simplify teardown.

[ ... ]

> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index de8ee3db43601..b785e098f7597 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -1188,6 +1188,9 @@ void dw_pcie_ep_deinit(struct dw_pcie_ep *ep)
>  {
>  	struct pci_epc *epc = ep->epc;
>  
> +	if (epc->init_complete)
> +		pci_epc_deinit_notify(epc);
> +

[Severity: High]
Can this unprotected check of epc->init_complete cause a TOCTOU race leading
to a double-free?

If the endpoint controller remove function calls dw_pcie_ep_deinit()
concurrently with a hardware interrupt like PERST calling
pci_epc_deinit_notify() directly, both threads could read
epc->init_complete as true without holding epc->list_lock.

Since pci_epc_deinit_notify() doesn't verify epc->init_complete under a
lock:

drivers/pci/endpoint/pci-epc-core.c:pci_epc_deinit_notify() {
    ...
		mutex_unlock(&epf->lock);
	}
	epc->init_complete = false;
	mutex_unlock(&epc->list_lock);
}

Could this result in duplicate calls to pci_epc_deinit_notify(), causing
EPF drivers like pci-epf-mhi to execute their epc_deinit callback twice
and double-free resources such as workqueues and caches?

>  	dw_pcie_ep_cleanup(ep);
>  
>  	pci_epc_mem_free_addr(epc, ep->msi_mem_phys, ep->msi_mem,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921142147.435953-1-lgs201920130244@gmail.com?part=1