Commit 1d45a705 authored by Gonglei's avatar Gonglei Committed by Andreas Färber
Browse files

qdev: Add cleanup logic in device_set_realized() to avoid resource leak 



At present, this function doesn't have partial cleanup implemented,
which will cause resource leaks in some scenarios.

Example:

1. Assume that "dc->realize(dev, &local_err)" executes successful
   and local_err == NULL;
2. device hotplug in hotplug_handler_plug() executes but fails
   (it is prone to occur). Then local_err != NULL;
3. error_propagate(errp, local_err) and return. But the resources
   which have been allocated in dc->realize() will be leaked.
Simple backtrace:
  dc->realize()
   |->device_realize
            |->pci_qdev_init()
                |->do_pci_register_device()
                |->etc.

Add fuller cleanup logic which assures that function can
goto appropriate error label as local_err population is
detected at each relevant point.

Signed-off-by: default avatarGonglei <arei.gonglei@huawei.com>
Reviewed-by: default avatarPeter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: default avatarAndreas Färber <afaerber@suse.de>
parent cd4520ad

hw/core/qdev.c

+38 −14
Original line number Diff line number Diff line
@@ -834,12 +834,14 @@ static void device_set_realized(Object *obj, bool value, Error **errp) 
            dc->realize(dev, &local_err);

        }



        if (dev->parent_bus && dev->parent_bus->hotplug_handler &&

            local_err == NULL) {

        if (local_err != NULL) {

            goto fail;

        }



        if (dev->parent_bus && dev->parent_bus->hotplug_handler) {

            hotplug_handler_plug(dev->parent_bus->hotplug_handler,

                                 dev, &local_err);

        } else if (local_err == NULL &&

                   object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {

        } else if (object_dynamic_cast(qdev_get_machine(), TYPE_MACHINE)) {

            HotplugHandler *hotplug_ctrl;

            MachineState *machine = MACHINE(qdev_get_machine());

            MachineClass *mc = MACHINE_GET_CLASS(machine);
@@ -852,21 +854,24 @@ static void device_set_realized(Object *obj, bool value, Error **errp) 
            }

        }



        if (qdev_get_vmsd(dev) && local_err == NULL) {

        if (local_err != NULL) {

            goto post_realize_fail;

        }



        if (qdev_get_vmsd(dev)) {

            vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,

                                           dev->instance_id_alias,

                                           dev->alias_required_for_version);

        }

        if (local_err == NULL) {



        QLIST_FOREACH(bus, &dev->child_bus, sibling) {

            object_property_set_bool(OBJECT(bus), true, "realized",

                                         &local_err);

            if (local_err != NULL) {

                    break;

                goto child_realize_fail;

            }

        }

        }

        if (dev->hotplugged && local_err == NULL) {

        if (dev->hotplugged) {

            device_reset(dev);

        }

        dev->pending_deleted_event = false;
@@ -888,11 +893,30 @@ static void device_set_realized(Object *obj, bool value, Error **errp) 
    }



    if (local_err != NULL) {

        error_propagate(errp, local_err);

        return;

        goto fail;

    }



    dev->realized = value;

    return;



child_realize_fail:

    QLIST_FOREACH(bus, &dev->child_bus, sibling) {

        object_property_set_bool(OBJECT(bus), false, "realized",

                                 NULL);

    }



    if (qdev_get_vmsd(dev)) {

        vmstate_unregister(dev, qdev_get_vmsd(dev), dev);

    }



post_realize_fail:

    if (dc->unrealize) {

        dc->unrealize(dev, NULL);

    }



fail:

    error_propagate(errp, local_err);

    return;

}



static bool device_get_hotpluggable(Object *obj, Error **errp)