Re: [PATCH 07/13] PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Unconditional teardown in brcm_pcie_remove on pci_host_probe failure causes UAF or NULL pointer dereference.
--

commit d3ea9196e71b4fdf62dbbe06f00b5b226f406396
Author: Jim Quinlan <james.quinlan@broadcom.com>

PCI: brcmstb: Declare and assign quirk 32BIT_PCI_OPS
    
Declare a new quirk that indicates SoCs where the config-space accesses are
restricted to 32bit R/W.

> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 8927d1d97f4a8..be414d3468555 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c

[ ... ]

> @@ -2180,8 +2182,8 @@ static int brcm_pcie_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	bridge->ops = pcie->cfg->soc_base == BCM7425 ?
> -				&brcm7425_pcie_ops : &brcm_pcie_ops;
> +	bridge->ops = BQUIRK(pcie, 32BIT_PCI_OPS)
> +		? &brcm_pcie_ops32 : &brcm_pcie_ops;
>  	bridge->sysdata = pcie;
>  
>  	platform_set_drvdata(pdev, pcie);

[Severity: High]
This isn't a bug introduced by this patch, but does this probe error path have
a use-after-free?

Right after the modified code in brcm_pcie_probe(), we have this error path:

	ret = pci_host_probe(bridge);
	if (!ret && !brcm_pcie_link_up(pcie))
		ret = -ENODEV;

	if (ret) {
		brcm_pcie_remove(pdev);
		return ret;
	}

If pci_host_probe() returns an error, it has already cleaned up and freed
bridge->bus.

When we then call brcm_pcie_remove(pdev), it unconditionally accesses the
freed pointer:

	pci_lock_rescan_remove();
	pci_stop_root_bus(bridge->bus);
	pci_remove_root_bus(bridge->bus);
	pci_unlock_rescan_remove();

Can this cause a kernel panic on device probe failure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911233541.1650895-1-james.quinlan@broadcom.com?part=7