Re: [PATCH v2 12/16] PCI: dwc: sophgo: Use cached PCIe capability offset

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing `.deinit` callback causes IRQ domain leak and leaves a dangling chained IRQ handler if `dw_pcie_host_init()` fails.
--

commit bec8b7e2c9e17ca16137f5895fed067563f850fa
Author: Hans Zhang <18255117159@163.com>

PCI: dwc: sophgo: Use cached PCIe capability offset

dw_pcie_host_init() calls .init before caching, so we must call
dw_pcie_get_pcie_cap() inside .init. The hardware is already enabled by
the driver's own initialization before this point. The helper will cache
the offset and avoid redundant searches.

> diff --git a/drivers/pci/controller/dwc/pcie-sophgo.c b/drivers/pci/controller/dwc/pcie-sophgo.c
> index 044088898819e..39703d2b7b5f2 100644
> --- a/drivers/pci/controller/dwc/pcie-sophgo.c
> +++ b/drivers/pci/controller/dwc/pcie-sophgo.c
> @@ -164,9 +164,10 @@ static void sophgo_pcie_msi_enable(struct dw_pcie_rp *pp)
>  static void sophgo_pcie_disable_l0s_l1(struct dw_pcie_rp *pp)
>  {
>  	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> -	u32 offset, val;
> +	u8 offset;
> +	u32 val;
>  
> -	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> +	offset = dw_pcie_get_pcie_cap(pci);
>  
>  	dw_pcie_dbi_ro_wr_en(pci);
>  

[Severity: High]
This is a pre-existing issue, but while reviewing this init sequence,
I noticed a potential resource leak and dangling pointer on the error path.

If dw_pcie_host_init() fails (e.g. due to a link training timeout), it invokes
the .deinit callback to tear down host-specific resources. However, the driver
is missing a .deinit callback:

drivers/pci/controller/dwc/pcie-sophgo.c:
static const struct dw_pcie_host_ops sophgo_pcie_host_ops = {
	.init = sophgo_pcie_host_init,
};

In sophgo_pcie_host_init(), an IRQ domain and a chained handler are set up:

drivers/pci/controller/dwc/pcie-sophgo.c:sophgo_pcie_host_init() {
    ...
	irq = sophgo_pcie_init_irq_domain(pp);
	if (irq < 0)
		return irq;

	irq_set_chained_handler_and_data(irq, sophgo_pcie_intx_handler, pp);
    ...
}

Because there is no .deinit callback, the IRQ domain and chained handler are
never removed when dw_pcie_host_init() fails.

When sophgo_pcie_probe() returns an error, devres will free the struct
sophgo_pcie (which contains pp).

If the parent interrupt fires later, could the dangling chained handler
(sophgo_pcie_intx_handler) execute and dereference the freed pp pointer,
leading to a use-after-free panic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260530153101.695580-1-18255117159@163.com?part=12