Re: [PATCH v8 14/14] PCI: tegra194: Fix CBB timeout caused by DBI access before core power-on

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Null pointer dereference in dwc_pcie_rasdes_debugfs_deinit() when RAS DES capability is absent. This problem wasn't introduced by this patch, but causes an unconditional crash on cleanup.
- [High] Unbounded memory leak of DMA coherent memory, EDMA controller structures, and debugfs elements on every PERST# deassertion. This problem wasn't introduced by this patch, but it causes a remotely triggerable DoS.
- [High] Double free and UAF of EDMA, debugfs, and PTM pointers due to missing pointer invalidation in cleanup paths. This problem wasn't introduced by this patch, but causes a kernel panic on repeated cleanup.
- [High] Unchecked return value of reset_control_deassert() leads to the identical CBB hardware timeout. This problem wasn't introduced by this patch, but the patch relies on the deassert succeeding.
- [High] Calling dw_pcie_ep_deinit() in module removal path triggers a CBB timeout. This problem wasn't introduced by this patch, but crashes the system on driver unbind.
--

--- Patch [14]: [PATCH v8 14/14] PCI: tegra194: Fix CBB timeout caused by DBI access before core power-on ---
commit 10ff75dbe7f3c6a826d07e1505268c730461b17c
Author: Manikanta Maddireddy <mmaddireddy@nvidia.com>
Subject: PCI: tegra194: Fix CBB timeout caused by DBI access before core power-on

This patch delays the endpoint cleanup operations in the TEGRA194 PCI driver
until after the core reset has been deasserted. By moving
pci_epc_deinit_notify() and dw_pcie_ep_cleanup() after
reset_control_deassert(), it ensures the dbi registers are accessible. This
prevents a cbb timeout that occurs when accessing registers on an unpowered
core.

> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c
> index b159836b7e9a0..b13f7221c9761 100644
> --- a/drivers/pci/controller/dwc/pcie-tegra194.c
> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c
> @@ -1749,10 +1749,6 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>  		goto fail_phy;
>  	}
>  
> -	/* Perform cleanup that requires refclk */
> -	pci_epc_deinit_notify(pcie->pci.ep.epc);
> -	dw_pcie_ep_cleanup(&pcie->pci.ep);
> -
>  	/* Clear any stale interrupt statuses */
>  	appl_writel(pcie, 0xFFFFFFFF, APPL_INTR_STATUS_L0);

[ ... ]

> @@ -1822,6 +1818,10 @@ static void pex_ep_event_pex_rst_deassert(struct tegra_pcie_dw *pcie)
>  
>  	reset_control_deassert(pcie->core_rst);

[Severity: High]
This is a pre-existing issue, but is it safe to proceed without checking the 
return value of reset_control_deassert() here?

If reset_control_deassert() fails and returns an error, the core remains 
unpowered and reset is still asserted. The code then proceeds to call 
dw_pcie_ep_cleanup() and access dbi registers.

Accessing the dbi registers on an unpowered core triggers the exact same cbb 
hardware timeout (bus error) that this patch intends to fix.

> +	/* Perform cleanup that requires refclk and core reset deasserted */
> +	pci_epc_deinit_notify(pcie->pci.ep.epc);
> +	dw_pcie_ep_cleanup(&pcie->pci.ep);

[Severity: High]
This isn't a bug introduced by this patch, but does repeatedly calling 
dw_pcie_ep_cleanup() followed by dw_pcie_ep_init_registers() cause a memory 
leak?

When the host repeatedly toggles the perst# signal, each deassertion calls 
dw_pcie_ep_init_registers(), which unconditionally allocates memory using 
devres/dmam functions (like dw_pcie_edma_ll_alloc(), dw_edma_probe(), and 
dwc_pcie_debugfs_init()).

Since dw_pcie_ep_cleanup() unregisters the subsystems but cannot free the 
devm_ allocated memory, this leads to an unbounded memory leak on every 
link toggle.

[Severity: High]
This is a pre-existing issue, but does dw_pcie_ep_cleanup() leave pointers 
dangling?

When pex_ep_event_pex_rst_deassert() calls dw_pcie_ep_cleanup(), it frees 
resources (like irqs via dw_edma_remove() and dentries via 
dwc_pcie_debugfs_deinit()), but leaves pointers dangling.

If dw_pcie_ep_init_registers() fails before updating these pointers, they 
remain dangling. A subsequent perst# deassertion will call 
dw_pcie_ep_cleanup() again on the dangling pointers, resulting in a double 
free of irqs and a use-after-free of dentries.

[Severity: Critical]
This is a pre-existing issue, but does dw_pcie_ep_cleanup() trigger a null 
pointer dereference when the rasdes capability is absent?

If the hardware lacks the rasdes capability, dwc_pcie_rasdes_debugfs_init() 
returns early without allocating rasdes_info, leaving 
pci->debugfs->rasdes_info as NULL.

When dw_pcie_ep_cleanup() runs (either on driver removal or perst# toggle), 
dwc_pcie_rasdes_debugfs_deinit() is called, which unconditionally dereferences 
pci->debugfs->rasdes_info to destroy the mutex:

drivers/pci/controller/dwc/pcie-designware-debugfs.c:dwc_pcie_rasdes_debugfs_deinit() {
    struct dwc_pcie_rasdes_info *rinfo = pci->debugfs->rasdes_info;
    
    mutex_destroy(&rinfo->reg_event_lock);
    ...
}

This will cause a kernel panic on cleanup.

[Severity: High]
This isn't a bug introduced by this patch, but does the driver module removal 
path also trigger a cbb timeout?

Looking at tegra_pcie_dw_remove() in drivers/pci/controller/dwc/pcie-tegra194.c, 
it calls pex_ep_event_pex_rst_assert(), which completely disables core clocks 
and asserts the core reset.

It then immediately calls dw_pcie_ep_deinit(), which invokes dw_edma_remove() 
and writes to eDMA hardware registers. Accessing registers while clocks are off 
and reset is asserted causes the same cbb timeout (bus error), hanging the 
system during module unbind.

> +
>  	val = dw_pcie_readl_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL);
>  	val &= ~PORT_LOGIC_SPEED_CHANGE;
>  	dw_pcie_writel_dbi(pci, PCIE_LINK_WIDTH_SPEED_CONTROL, val);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260324190755.1094879-1-mmaddireddy@nvidia.com?part=14