From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59508) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y9Gz9-00032w-Ph for qemu-devel@nongnu.org; Thu, 08 Jan 2015 12:35:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y9Gz3-000614-3z for qemu-devel@nongnu.org; Thu, 08 Jan 2015 12:35:03 -0500 Received: from e7.ny.us.ibm.com ([32.97.182.137]:54949) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y9Gz2-00060n-WF for qemu-devel@nongnu.org; Thu, 08 Jan 2015 12:34:57 -0500 Received: from /spool/local by e7.ny.us.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 8 Jan 2015 12:34:56 -0500 From: Michael Roth Date: Thu, 8 Jan 2015 11:33:06 -0600 Message-Id: <1420738472-23267-3-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1420738472-23267-1-git-send-email-mdroth@linux.vnet.ibm.com> References: <1420738472-23267-1-git-send-email-mdroth@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Subject: [Qemu-devel] [PATCH 02/88] qdev: Add cleanup logic in device_set_realized() to avoid resource leak List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org From: Gonglei 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 Reviewed-by: Peter Crosthwaite Cc: qemu-stable@nongnu.org Signed-off-by: Andreas Färber (cherry picked from commit 1d45a705fc007a13f20d18473290082eae6d1725) Signed-off-by: Michael Roth --- hw/core/qdev.c | 52 ++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/hw/core/qdev.c b/hw/core/qdev.c index 1a88865..0e21fad 100644 --- a/hw/core/qdev.c +++ b/hw/core/qdev.c @@ -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", + + 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; @@ -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) -- 1.9.1