On Thu, 13 Jun 2019 09:56:27 +1000 David Gibson <da...@gibson.dropbear.id.au> wrote:
> On Thu, Jun 13, 2019 at 01:02:02AM +0200, Philippe Mathieu-Daudé wrote:
> > Commit 14e714900f6 refactored the call to spapr_dt_drc(),
> > but used an incorrect object owner as argument.
> >
> > This fixes:
> >
> > /hw/ppc/spapr_pci.c: 1367 in spapr_dt_pci_bus()
> > >>> CID 1401933: Null pointer dereferences (FORWARD_NULL)
> > >>> Dereferencing null pointer "bus".
> > 1367 ret = spapr_dt_drc(fdt, offset, OBJECT(bus->parent_dev),
> > 1368 SPAPR_DR_CONNECTOR_TYPE_PCI);
> >
> > Fixes: 14e714900f6
> > Reported-by: Coverity (CID 1401933)
> > Signed-off-by: Philippe Mathieu-Daudé <phi...@redhat.com>
>
> Nack. The bus bridge is the correct owner in most cases here. It
> *used* to be that all the PCI DRCs were owned by the PHB, but that's
> intentionally not the case with the changes to allow hotplug under P2P
> bridges.
>
> AFAICT, the bus parameter does have to be non-NULL from both callers,
> so I think the correct fix is to remove the test on if (bus) earlier
> in the function.
>
... and maybe make this obvious with an assert(bus) ?
> > ---
> > hw/ppc/spapr_pci.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index 957ae88bbd..e0cd3f11f1 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1364,8 +1364,7 @@ static int spapr_dt_pci_bus(SpaprPhbState *sphb,
> > PCIBus *bus,
> > }
> > }
> >
> > - ret = spapr_dt_drc(fdt, offset, OBJECT(bus->parent_dev),
> > - SPAPR_DR_CONNECTOR_TYPE_PCI);
> > + ret = spapr_dt_drc(fdt, offset, OBJECT(sphb),
> > SPAPR_DR_CONNECTOR_TYPE_PCI);
> > if (ret) {
> > return ret;
> > }
>
pgp2ufSg9uSnq.pgp
Description: OpenPGP digital signature
