[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/8] qdev: fix object reference leak in case dev
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH 4/8] qdev: fix object reference leak in case device.realize() fails |
Date: |
Fri, 22 Jul 2016 11:39:18 +1000 |
User-agent: |
Mutt/1.6.2 (2016-07-01) |
On Thu, Jul 21, 2016 at 05:54:35PM +0200, Igor Mammedov wrote:
> If device doesn't have parent assined before its realize
> is called, device_set_realized() will implicitly set parent
> to '/machine/unattached'.
>
> However device_set_realized() may fail after that point at
> several other points leaving not realized object dangling
> in '/machine/unattached' and as result caller of
>
> obj = object_new()
> obj->ref == 1
> object_property_set_bool(obj,..., true, "realized",...)
> obj->ref == 2
> if (fail)
> object_unref(obj);
> obj->ref == 1
>
> will get object leak instead of expected object destruction.
>
> Fix it by making device_set_realized() to cleanup after itself
> in case of failure.
>
> Signed-off-by: Igor Mammedov <address@hidden>
Reviewed-by: David Gibson <address@hidden>
> ---
> hw/core/qdev.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 6680089..ee4a083 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -885,6 +885,8 @@ static void device_set_realized(Object *obj, bool value,
> Error **errp)
> HotplugHandler *hotplug_ctrl;
> BusState *bus;
> Error *local_err = NULL;
> + bool unattached_parent = false;
> + static int unattached_count;
>
> if (dev->hotplugged && !dc->hotpluggable) {
> error_setg(errp, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
> @@ -893,12 +895,12 @@ static void device_set_realized(Object *obj, bool
> value, Error **errp)
>
> if (value && !dev->realized) {
> if (!obj->parent) {
> - static int unattached_count;
> gchar *name = g_strdup_printf("device[%d]", unattached_count++);
>
> object_property_add_child(container_get(qdev_get_machine(),
> "/unattached"),
> name, obj, &error_abort);
> + unattached_parent = true;
> g_free(name);
> }
>
> @@ -987,6 +989,10 @@ post_realize_fail:
>
> fail:
> error_propagate(errp, local_err);
> + if (unattached_parent) {
> + object_unparent(OBJECT(dev));
> + unattached_count--;
> + }
> }
>
> static bool device_get_hotpluggable(Object *obj, Error **errp)
--
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 2/8] exec: don't use cpu_index to detect if cpu_exec_init()'s been called for cpu, (continued)
[Qemu-devel] [PATCH 1/8] exec: reduce CONFIG_USER_ONLY ifdeffenery, Igor Mammedov, 2016/07/21
[Qemu-devel] [PATCH 3/8] exec: set cpu_index only if it's been explictly set, Igor Mammedov, 2016/07/21
[Qemu-devel] [PATCH 5/8] pc: init CPUState->cpu_index with index in possible_cpus[], Igor Mammedov, 2016/07/21
[Qemu-devel] [PATCH 4/8] qdev: fix object reference leak in case device.realize() fails, Igor Mammedov, 2016/07/21
- Re: [Qemu-devel] [PATCH 4/8] qdev: fix object reference leak in case device.realize() fails,
David Gibson <=
[Qemu-devel] [PATCH 6/8] spapr: init CPUState->cpu_index with index relative to core-id, Igor Mammedov, 2016/07/21
Re: [Qemu-devel] [PATCH 6/8] spapr: init CPUState->cpu_index with index relative to core-id, David Gibson, 2016/07/26
[Qemu-devel] [PATCH 7/8] Revert "pc: Enforce adding CPUs contiguously and removing them in opposite order", Igor Mammedov, 2016/07/21
[Qemu-devel] [PATCH 8/8] Revert "spapr: Ensure CPU cores are added contiguously and removed in LIFO order", Igor Mammedov, 2016/07/21
Re: [Qemu-devel] [PATCH 0/8] Fix migration issues with arbitrary cpu-hot(un)plug, Michael S. Tsirkin, 2016/07/21