Skip to content

Commit

Permalink
qdev: Add cleanup logic in device_set_realized() to avoid resource leak
Browse files Browse the repository at this point in the history
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: Gonglei <arei.gonglei@huawei.com>
Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Andreas Färber <afaerber@suse.de>
(cherry picked from commit 1d45a70)
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
  • Loading branch information
gongleiarei authored and mdroth committed Dec 24, 2014
1 parent 8bb90ee commit d6af26d
Showing 1 changed file with 38 additions and 14 deletions.
52 changes: 38 additions & 14 deletions hw/core/qdev.c
Expand Up @@ -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);
Expand All @@ -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",

QLIST_FOREACH(bus, &dev->child_bus, sibling) {
object_property_set_bool(OBJECT(bus), true, "realized",
&local_err);
if (local_err != NULL) {
break;
}
if (local_err != NULL) {
goto child_realize_fail;
}
}
if (dev->hotplugged && local_err == NULL) {
if (dev->hotplugged) {
device_reset(dev);
}
dev->pending_deleted_event = false;
Expand All @@ -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)
Expand Down

0 comments on commit d6af26d

Please sign in to comment.