Re: [Qemu-devel] [PATCH] hw/pci: fix crash on shpc error flow

[Amos Kong]

On Wed, Nov 05, 2014 at 04:29:35PM +0200, Marcel Apfelbaum wrote:
> If the pci bridge enters in error flow as part
> of init process it will only delete the shpc mmio
> subregion but not remove it from the properties list,
> resulting in segmentation fault when the bridge runs
> the exit function.
> 
> Example: add a pci bridge without specifing the chassis number:
>     <qemu-bin> ... -device pci-bridge,id=p1
> Result:
>     (qemu) qemu-system-x86_64: -device pci-bridge,id=p1: Bridge chassis not 
> specified. Each bridge is required to be assigned a unique chassis id > 0.
>     qemu-system-x86_64: -device pci-bridge,id=p1: Device
>     initialization failed.
>     Segmentation fault (core dumped)
> 
>     if (child->class->unparent) {
>     #0  0x00005555558d629b in object_finalize_child_property 
> (obj=0x555556d2e830, name=0x555556d30630 "shpc-mmio[0]", 
> opaque=0x555556a42fc8) at qom/object.c:1078
>     #1  0x00005555558d4b1f in object_property_del_all (obj=0x555556d2e830) at 
> qom/object.c:367
>     #2  0x00005555558d4ca1 in object_finalize (data=0x555556d2e830) at 
> qom/object.c:412
>     #3  0x00005555558d55a1 in object_unref (obj=0x555556d2e830) at 
> qom/object.c:720
>     #4  0x000055555572c907 in qdev_device_add (opts=0x5555563544f0) at 
> qdev-monitor.c:566
>     #5  0x0000555555744f16 in device_init_func (opts=0x5555563544f0, 
> opaque=0x0) at vl.c:2213
>     #6  0x00005555559cf5f0 in qemu_opts_foreach (list=0x555555e0f8e0 
> <qemu_device_opts>, func=0x555555744efa <device_init_func>, opaque=0x0, 
> abort_on_failure=1) at util/qemu-option.c:1057
>     #7  0x000055555574a11b in main (argc=16, argv=0x7fffffffdde8, 
> envp=0x7fffffffde70) at vl.c:423
> 
> Unparent the shpc mmio region as part of shpc cleanup.
> 
> Signed-off-by: Marcel Apfelbaum <address@hidden>
> ---
>  hw/pci/shpc.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 65b2f51..2e887d7 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -662,6 +662,7 @@ void shpc_cleanup(PCIDevice *d, MemoryRegion *bar)
>      SHPCDevice *shpc = d->shpc;
>      d->cap_present &= ~QEMU_PCI_CAP_SHPC;
>      memory_region_del_subregion(bar, &shpc->mmio);
> +    object_unparent(OBJECT(&shpc->mmio));
>      /* TODO: cleanup config space changes? */
>      g_free(shpc->config);
>      g_free(shpc->cmask);
> -- 


Reviewed-by: Amos Kong <address@hidden>

> 1.8.3.1
> 

-- 
                        Amos.

signature.asc
Description: Digital signature