From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37687) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bQR3O-0006sA-1Q for qemu-devel@nongnu.org; Thu, 21 Jul 2016 23:23:11 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bQR3M-0003td-EU for qemu-devel@nongnu.org; Thu, 21 Jul 2016 23:23:09 -0400 Date: Fri, 22 Jul 2016 11:39:18 +1000 From: David Gibson Message-ID: <20160722013918.GE15941@voom.fritz.box> References: <1469116479-233280-1-git-send-email-imammedo@redhat.com> <1469116479-233280-5-git-send-email-imammedo@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="vv4Sf/kQfcwinyKX" Content-Disposition: inline In-Reply-To: <1469116479-233280-5-git-send-email-imammedo@redhat.com> Subject: Re: [Qemu-devel] [PATCH 4/8] qdev: fix object reference leak in case device.realize() fails List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Igor Mammedov Cc: qemu-devel@nongnu.org, Paolo Bonzini , Peter Crosthwaite , Richard Henderson , Eduardo Habkost , "Michael S. Tsirkin" , Alexander Graf , Riku Voipio , Bharata B Rao , qemu-ppc@nongnu.org --vv4Sf/kQfcwinyKX Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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'. >=20 > 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 >=20 > obj =3D object_new() > obj->ref =3D=3D 1 > object_property_set_bool(obj,..., true, "realized",...) > obj->ref =3D=3D 2 > if (fail) > object_unref(obj); > obj->ref =3D=3D 1 >=20 > will get object leak instead of expected object destruction. >=20 > Fix it by making device_set_realized() to cleanup after itself > in case of failure. >=20 > Signed-off-by: Igor Mammedov Reviewed-by: David Gibson > --- > hw/core/qdev.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) >=20 > 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 val= ue, Error **errp) > HotplugHandler *hotplug_ctrl; > BusState *bus; > Error *local_err =3D NULL; > + bool unattached_parent =3D false; > + static int unattached_count; > =20 > 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 v= alue, Error **errp) > =20 > if (value && !dev->realized) { > if (!obj->parent) { > - static int unattached_count; > gchar *name =3D g_strdup_printf("device[%d]", unattached_cou= nt++); > =20 > object_property_add_child(container_get(qdev_get_machine(), > "/unattached"), > name, obj, &error_abort); > + unattached_parent =3D true; > g_free(name); > } > =20 > @@ -987,6 +989,10 @@ post_realize_fail: > =20 > fail: > error_propagate(errp, local_err); > + if (unattached_parent) { > + object_unparent(OBJECT(dev)); > + unattached_count--; > + } > } > =20 > static bool device_get_hotpluggable(Object *obj, Error **errp) --=20 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 --vv4Sf/kQfcwinyKX Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXkXlGAAoJEGw4ysog2bOSJoQP+wSQkrGKpd+M+PeW6A1TNLm/ DT6v/G73yWeQzpKF/FRpiQeLUEYjo1PeNnIvLW2EvorpJe8J3YXUmMJeXF7+I/di 2ZovWmgZcOcTfSVeINcadFI/0WbMmWEDLUYEegn8R9r1yf5BnwkaVeLp8T0uQx6F jxaSNbbrUC3qGMat+zJIPnRJdHqOBMh6lNv8FzjIjkCeVoGa74tbctOJpSpvH1HD P6OlZnEUIljB2Gb340OD7Vd7tJ0Wz/pJh9KFx0mMfZKmEwkOT9stU8+XLsgluOBP YS77Iwz0eHc4xc7tVyUqGD/N4DSugbX/hQ0H0gvfN1KCcPO1WTIs9W1UZil3GPBU VFj+Qoq2Vp8DNZfiQh8oXIo1rwJbQszW7NojhAnk/d+mkDDHyZqCQAbwMnFJjOik 09C9Cxoc94zfdNxVvRvykpvpeLQbVX9NjMQMMeoRHqubvEmPyZwXUbmj5Cle9U38 65LcyDVeJwraLiU9ISGY7Vt1CrVG+9bUZmTMVHHvUksva0MfenhZUyJj9uSnBWsK ieRX8/AhrMGlNHprOeq2bQBI2KC1eonrdqsLwPtPkAbjxfXPCDZq6qX8sEhGiWVW 0W2If/BpixZDN/xWguWP/NTTLBCxXDiH2xupF24DSFfc86Nm5qxAJeBarMT9GKWp xovoTNrU8a4C585XuYjX =uBWC -----END PGP SIGNATURE----- --vv4Sf/kQfcwinyKX--