[PATCH v3 06/16] PCI: dwc: rockchip: Use cached PCIe capability offset
sashiko-bot at kernel.org
sashiko-bot at kernel.org
Mon Jul 20 08:21:57 PDT 2026
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The DWC core code unconditionally uses `pci->pcie_cap` without checking if it is zero, leading to silent PCI configuration space corruption, particularly in endpoint (EP) mode where it remains uninitialized for many drivers.
- [Low] An unused variable and redundant DBI read were left behind in `dw_pcie_link_get_max_link_width()` during the `pci->pcie_cap` refactoring.
Pre-existing issues:
- [High] Missing cleanup of chained IRQ handler and IRQ domain on probe failure causes Use-After-Free.
--
--- Patch [6]: [PATCH v3 06/16] PCI: dwc: rockchip: Use cached PCIe capability offset ---
commit 66fc930727ddec47d08c4dc09e796cb0d73aa5a9
Author: Hans Zhang <18255117159 at 163.com>
PCI: dwc: rockchip: Use cached PCIe capability offset
dw_pcie_host_init() calls .init before caching the offset. So inside
.init we call dw_pcie_get_pcie_cap() to trigger caching. The helper will
perform the DBI read (hardware is already enabled) and cache the result.
> diff --git a/drivers/pci/controller/dwc/pcie-dw-rockchip.c b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> index 731d93663ccae..be8b6187913df 100644
> --- a/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> +++ b/drivers/pci/controller/dwc/pcie-dw-rockchip.c
> @@ -369,7 +369,7 @@ static void rockchip_pcie_enable_l0s(struct dw_pcie *pci)
> u32 cap, lnkcap;
>
> /* Enable L0S capability for all SoCs */
> - cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
> + cap = dw_pcie_get_pcie_cap(pci);
> if (cap) {
> lnkcap = dw_pcie_readl_dbi(pci, cap + PCI_EXP_LNKCAP);
> lnkcap |= PCI_EXP_LNKCAP_ASPM_L0S;
[Severity: High]
This is a pre-existing issue, but does this code leave a dangling pointer and
cause a use-after-free on probe failure?
drivers/pci/controller/dwc/pcie-dw-rockchip.c:
rockchip_pcie_host_init() {
...
irq_set_chained_handler_and_data(irq, rockchip_pcie_intx_handler,
rockchip);
...
}
The chained handler is registered with the rockchip devres-managed structure.
If dw_pcie_host_init() fails later in the probe process (for example, while
waiting for the link to come up), the devres framework frees the rockchip
structure.
Since rockchip_pcie_host_ops lacks a .deinit callback to unregister the
chained handler and free the domain, will the handler dereference the freed
rockchip struct if the legacy IRQ fires afterward?
[Severity: High]
Can this corrupt the PCI configuration space if pci->pcie_cap is 0?
drivers/pci/controller/dwc/pcie-designware-ep.c:
dw_pcie_ep_init_non_sticky_registers() {
...
dw_pcie_setup(pci);
...
}
drivers/pci/controller/dwc/pcie-designware.c:
dw_pcie_link_set_max_speed() {
...
u8 offset = pci->pcie_cap;
cap = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
...
}
If an EP driver does not call dw_pcie_get_pcie_cap() during its .init
callback, pci->pcie_cap remains 0. The dw_pcie_ep_init_non_sticky_registers()
function unconditionally calls dw_pcie_setup() without verifying if
pci->pcie_cap was initialized.
Does this blindly use offset 0 instead of the PCIe capability structure,
permanently corrupting critical PCI config registers (like Cache Line Size,
Latency Timer, Header Type, or BIST) when offset 0 is accessed?
This also seems contrary to the commit message's claim that a warning would
be printed and the operation skipped, as the DWC core code uses the offset
unconditionally.
[Severity: Low]
Is an unused variable left behind, causing a redundant DBI read?
drivers/pci/controller/dwc/pcie-designware.c:
dw_pcie_link_get_max_link_width() {
u8 cap = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
u32 lnkcap = dw_pcie_readl_dbi(pci, pci->pcie_cap + PCI_EXP_LNKCAP);
...
}
The cap variable is initialized via dw_pcie_find_capability() but is never
used, while the function proceeds to use pci->pcie_cap.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720150619.24733-1-18255117159@163.com?part=6
More information about the linux-amlogic
mailing list