| Message ID | 20240328180126.23574-1-amishin@t-argos.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: dwc: Fix potential NULL dereference | expand |
The subject line should be: PCI: al: ... since this fix is specific to the "al" driver, not generic to "dwc". On Thu, Mar 28, 2024 at 09:01:26PM +0300, Aleksandr Mishin wrote: > In al_pcie_config_prepare() resource_list_first_type() may return > NULL which is later dereferenced. Fix this bug by adding NULL check. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources") > Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru> > --- > drivers/pci/controller/dwc/pcie-al.c | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c > index 6dfdda59f328..29bc99d48295 100644 > --- a/drivers/pci/controller/dwc/pcie-al.c > +++ b/drivers/pci/controller/dwc/pcie-al.c > @@ -252,7 +252,12 @@ static void al_pcie_config_prepare(struct al_pcie *pcie) > u8 secondary_bus; > u32 cfg_control; > u32 reg; > - struct resource *bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res; > + > + struct resource_entry *ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS); > + if (!ft) > + return; I don't think this is right. If we don't have an IORESOURCE_BUS resource and we just silently return here, we will not write the CFG_CONTROL register. It looks essential that CFG_CONTROL be set, so if we can't do that, the .probe() should fail. But I think we are actually guaranteed that there is an IORESOURCE_BUS resource because this path fabricates one if the "bus-range" DT property doesn't exist: al_pcie_probe dw_pcie_host_init devm_pci_alloc_host_bridge devm_of_pci_bridge_init pci_parse_request_of_pci_ranges devm_of_pci_get_host_bridge_resources err = of_pci_parse_bus_range if (err) bus_range->flags = IORESOURCE_BUS # <-- I wouldn't necessarily object to doing something like other drivers do: gen_pci_init bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS); if (!bus) return ERR_PTR(-ENODEV); xilinx_cpm_pcie_probe bus = resource_list_first_type(&bridge->windows, IORESOURCE_BUS); if (!bus) return -ENODEV; But it would have to lead to .probe() failing, not just a silent skipping of CFG_CONTROL setup. > + struct resource *bus = ft->res; > > target_bus_cfg = &pcie->target_bus_cfg; > > -- > 2.30.2 >
diff --git a/drivers/pci/controller/dwc/pcie-al.c b/drivers/pci/controller/dwc/pcie-al.c index 6dfdda59f328..29bc99d48295 100644 --- a/drivers/pci/controller/dwc/pcie-al.c +++ b/drivers/pci/controller/dwc/pcie-al.c @@ -252,7 +252,12 @@ static void al_pcie_config_prepare(struct al_pcie *pcie) u8 secondary_bus; u32 cfg_control; u32 reg; - struct resource *bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res; + + struct resource_entry *ft = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS); + if (!ft) + return; + + struct resource *bus = ft->res; target_bus_cfg = &pcie->target_bus_cfg;
In al_pcie_config_prepare() resource_list_first_type() may return NULL which is later dereferenced. Fix this bug by adding NULL check. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: 0f71c60ffd26 ("PCI: dwc: Remove storing of PCI resources") Signed-off-by: Aleksandr Mishin <amishin@t-argos.ru> --- drivers/pci/controller/dwc/pcie-al.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)