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

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the INTx IRQ domain and chained handler are leaked on probe failure, which leaves a dangling pointer in the active IRQ handler and leads to a use-after-free if the interrupt fires.
--

--- Patch [12]: [PATCH v3 12/16] PCI: dwc: sophgo: Use cached PCIe capability offset ---
commit 2e86ab7dbd69d3bcfaa3d7b4f601df27c48317f3
Author: Hans Zhang <[email protected]>
PCI: dwc: sophgo: Use cached PCIe capability offset

This commit modifies sophgo_pcie_disable_l0s_l1() to use the cached PCIe
capability offset rather than searching for it again. The offset is fetched
via dw_pcie_get_pcie_cap() inside the .init callback.

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

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the call
chain from sophgo_pcie_host_init() to sophgo_pcie_disable_l0s_l1(), I
noticed a potential issue with how the INTx IRQ domain is handled during
probe failures.

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

        irq = sophgo_pcie_init_irq_domain(pp);
        if (irq < 0)
                return irq;

        irq_set_chained_handler_and_data(irq, sophgo_pcie_intx_handler, pp);

If dw_pcie_host_init() subsequently fails during sophgo_pcie_probe(), devres
will free the struct sophgo_pcie structure, including the pp pointer.

However, there is no .deinit callback in sophgo_pcie_host_ops to call
irq_domain_remove() and irq_set_chained_handler_and_data(irq, NULL, NULL).

Could this leave a dangling pointer in the active IRQ handler, leading to
a use-after-free if the interrupt fires?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic