Re: [Qemu-devel] [PATCH] spapr_pci: fix potential NULL pointer dereference
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 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); > -- David Gibson| I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson signature.asc Description: PGP signature
Re: [Qemu-devel] [PATCH] spapr_pci: fix potential NULL pointer dereference
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 Reviewed-by: Cédric Le Goater 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); >
[Qemu-devel] [PATCH] spapr_pci: fix potential NULL pointer dereference
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 --- 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);