Message ID | 153512460418.276612.14744411474887601809.stgit@bahia |
---|---|
State | New |
Headers | show |
Series | spapr_pci: fix potential NULL pointer dereference | expand |
On 08/24/2018 05:30 PM, Greg Kurz wrote: > Commit 2c88b098e76fd added a call to SPAPR_MACHINE_GET_CLASS(spapr) in > spapr_phb_realize() before we check spapr isn't NULL. This causes QEMU > to crash when starting a non-pseries machine with a sPAPR PHB. > > This could be fixed by setting the smc variable after the null check, > but it seems more explicit to use a ternary operator to skip the call > to SPAPR_MACHINE_GET_CLASS() if spapr is NULL, since spapr_phb_realize() > will return immediately in this case. > > This was reported by Coverity (CID 1395170 and 1395183). > > Fixes: 2c88b098e76fde0c7fcc0476dd3f80ce58409505 > Signed-off-by: Greg Kurz <groug@kaod.org> Reviewed-by: Cédric Le Goater <clg@kaod.org> Thanks, C. > --- > hw/ppc/spapr_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 5cd676e4430d..6bcb4f419b6b 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1559,7 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > sPAPRMachineState *spapr = > (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), > TYPE_SPAPR_MACHINE); > - sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > + sPAPRMachineClass *smc = spapr ? SPAPR_MACHINE_GET_CLASS(spapr) : NULL; > SysBusDevice *s = SYS_BUS_DEVICE(dev); > sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s); > PCIHostState *phb = PCI_HOST_BRIDGE(s); >
On Fri, Aug 24, 2018 at 05:30:04PM +0200, Greg Kurz wrote: > Commit 2c88b098e76fd added a call to SPAPR_MACHINE_GET_CLASS(spapr) in > spapr_phb_realize() before we check spapr isn't NULL. This causes QEMU > to crash when starting a non-pseries machine with a sPAPR PHB. > > This could be fixed by setting the smc variable after the null check, > but it seems more explicit to use a ternary operator to skip the call > to SPAPR_MACHINE_GET_CLASS() if spapr is NULL, since spapr_phb_realize() > will return immediately in this case. > > This was reported by Coverity (CID 1395170 and 1395183). > > Fixes: 2c88b098e76fde0c7fcc0476dd3f80ce58409505 > Signed-off-by: Greg Kurz <groug@kaod.org> Applied, thanks. > --- > hw/ppc/spapr_pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c > index 5cd676e4430d..6bcb4f419b6b 100644 > --- a/hw/ppc/spapr_pci.c > +++ b/hw/ppc/spapr_pci.c > @@ -1559,7 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) > sPAPRMachineState *spapr = > (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), > TYPE_SPAPR_MACHINE); > - sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); > + sPAPRMachineClass *smc = spapr ? SPAPR_MACHINE_GET_CLASS(spapr) : NULL; > SysBusDevice *s = SYS_BUS_DEVICE(dev); > sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s); > PCIHostState *phb = PCI_HOST_BRIDGE(s); >
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c index 5cd676e4430d..6bcb4f419b6b 100644 --- a/hw/ppc/spapr_pci.c +++ b/hw/ppc/spapr_pci.c @@ -1559,7 +1559,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp) sPAPRMachineState *spapr = (sPAPRMachineState *) object_dynamic_cast(qdev_get_machine(), TYPE_SPAPR_MACHINE); - sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(spapr); + sPAPRMachineClass *smc = spapr ? SPAPR_MACHINE_GET_CLASS(spapr) : NULL; SysBusDevice *s = SYS_BUS_DEVICE(dev); sPAPRPHBState *sphb = SPAPR_PCI_HOST_BRIDGE(s); PCIHostState *phb = PCI_HOST_BRIDGE(s);
Commit 2c88b098e76fd added a call to SPAPR_MACHINE_GET_CLASS(spapr) in spapr_phb_realize() before we check spapr isn't NULL. This causes QEMU to crash when starting a non-pseries machine with a sPAPR PHB. This could be fixed by setting the smc variable after the null check, but it seems more explicit to use a ternary operator to skip the call to SPAPR_MACHINE_GET_CLASS() if spapr is NULL, since spapr_phb_realize() will return immediately in this case. This was reported by Coverity (CID 1395170 and 1395183). Fixes: 2c88b098e76fde0c7fcc0476dd3f80ce58409505 Signed-off-by: Greg Kurz <groug@kaod.org> --- hw/ppc/spapr_pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)