Re: New Defects reported by Coverity Scan for Linux

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Nov 11, 2020 at 09:34:10AM -0600, Rob Herring wrote:
> On Tue, Nov 10, 2020 at 5:36 PM Gustavo Pimentel
> <Gustavo.Pimentel@synopsys.com> wrote:
> > On Tue, Nov 10, 2020 at 17:16:41, Bjorn Helgaas <helgaas@kernel.org>
> > wrote:
> >
> > > New Coverity complaint about v5.10-rc3, resulting from 9fff3256f93d
> > > ("PCI: dwc: Restore ATU memory resource setup to use last entry").
> > >
> > > I didn't try to figure out if this is real or a false positive, so
> > > just FYI.
> > >
> > > ----- Forwarded message from scan-admin@coverity.com -----
> > >
> > > Date: Mon, 09 Nov 2020 11:13:37 +0000 (UTC)
> > > From: scan-admin@coverity.com
> > > To: bjorn@helgaas.com
> > > Subject: New Defects reported by Coverity Scan for Linux
> > > Message-ID: <5fa924618fb3b_a62932acac7322f5033088@prd-scan-dashboard-0.mail>
> > >
> > > ** CID 1469110:  Null pointer dereferences  (FORWARD_NULL)
> > > /drivers/pci/controller/dwc/pcie-designware-host.c: 596 in dw_pcie_setup_rc()
> > >
> > > ________________________________________________________________________________________________________
> > > *** CID 1469110:  Null pointer dereferences  (FORWARD_NULL)
> > > /drivers/pci/controller/dwc/pcie-designware-host.c: 596 in dw_pcie_setup_rc()
> > > 590
> > > 591                   /* Get last memory resource entry */
> > > 592                   resource_list_for_each_entry(tmp, &pp->bridge->windows)
> > > 593                           if (resource_type(tmp->res) == IORESOURCE_MEM)
> >
> > Can the pp->bridge->windows list be empty in a typical use case?
> 
> Only if the DT has missing/malformed 'ranges'. 'ranges' is required to
> have any memory or i/o space, so we would error out before this point.

There are a lot of paths that lead here, and it's an awful lot of work
to verify that they all correctly error out if 'ranges' is invalid.

It would really be nice if we could structure this in such a way that
local analysis could show that we never dereference a null pointer
here.

I wouldn't want to uglify the code unnecessarily, but if a small code
change could avoid this false positive, I think it might be worth
doing.

Bjorn