From nobody Mon Apr 29 07:33:44 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1523027752123708.503845712511; Fri, 6 Apr 2018 08:15:52 -0700 (PDT) Received: from localhost ([::1]:33765 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4T5c-0002MN-0G for importer@patchew.org; Fri, 06 Apr 2018 11:15:44 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4T4b-0001im-7Y for qemu-devel@nongnu.org; Fri, 06 Apr 2018 11:14:42 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f4T4Y-0002Dt-0N for qemu-devel@nongnu.org; Fri, 06 Apr 2018 11:14:41 -0400 Received: from 6.mo7.mail-out.ovh.net ([188.165.39.218]:48913) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f4T4X-000295-P1 for qemu-devel@nongnu.org; Fri, 06 Apr 2018 11:14:37 -0400 Received: from player759.ha.ovh.net (unknown [10.109.122.48]) by mo7.mail-out.ovh.net (Postfix) with ESMTP id EEA1E96C90 for ; Fri, 6 Apr 2018 17:14:29 +0200 (CEST) Received: from bahia.lan (lns-bzn-46-82-253-208-248.adsl.proxad.net [82.253.208.248]) (Authenticated sender: groug@kaod.org) by player759.ha.ovh.net (Postfix) with ESMTPA id 5D50D6400B4; Fri, 6 Apr 2018 17:14:25 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Fri, 06 Apr 2018 17:14:19 +0200 Message-ID: <152302765908.144052.14302567702608397806.stgit@bahia.lan> User-Agent: StGit/0.17.1-46-g6855-dirty MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Ovh-Tracer-Id: 10292132523540715831 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedtgedrgeekgdekkecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 188.165.39.218 Subject: [Qemu-devel] [PATCH for-2.12] vfio-ccw: fix memory leak in vfio_ccw_realize() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Alex Williamson , Cornelia Huck , qemu-s390x@nongnu.org, qemu-stable@nongnu.org Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 The vfio_ccw_realize() function currently leaks vcdev->vdev.name if the subchannel is already attached or if vfio_get_device() fails. This happens because vcdev->vdev.name is expected to be freed in vfio_put_device() which isn't called in this case. Adding g_free(vcdev->vdev.name) on these two error paths would be enough to fix the leak, but this is unfortunate since we would then have three locations doing this. Also, this would be inconsistent with the label based rollback scheme of this function. The root issue is that vcdev->vdev.name is set before vfio_get_device() is called, which theoretically prevents to call vfio_put_device() to do the freeing. Well actually, we could call it anyway because vfio_put_base_device() is a nop if the device isn't attached, but this would be confusing. This patch hence moves all the logic of attaching the device to a separate vfio_ccw_get_device() function, which is the counterpart of vfio_put_device(). While here, vfio_put_device() is renamed to vfio_ccw_put_device() for consistency. Signed-off-by: Greg Kurz --- hw/vfio/ccw.c | 54 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 36 insertions(+), 18 deletions(-) diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 4e5855741a64..49ae986d288d 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -292,12 +292,44 @@ static void vfio_ccw_put_region(VFIOCCWDevice *vcdev) g_free(vcdev->io_region); } =20 -static void vfio_put_device(VFIOCCWDevice *vcdev) +static void vfio_ccw_put_device(VFIOCCWDevice *vcdev) { g_free(vcdev->vdev.name); vfio_put_base_device(&vcdev->vdev); } =20 +static int vfio_ccw_get_device(VFIOGroup *group, VFIOCCWDevice *vcdev, + Error **errp) +{ + char *name =3D g_strdup_printf("%x.%x.%04x", vcdev->cdev.hostid.cssid, + vcdev->cdev.hostid.ssid, + vcdev->cdev.hostid.devid); + VFIODevice *vbasedev; + + QLIST_FOREACH(vbasedev, &group->device_list, next) { + if (strcmp(vbasedev->name, name) =3D=3D 0) { + error_setg(errp, "vfio: subchannel %s has already been attache= d", + name); + goto out_err; + } + } + + if (vfio_get_device(group, vcdev->cdev.mdevid, &vcdev->vdev, errp)) { + goto out_err; + } + + vcdev->vdev.ops =3D &vfio_ccw_ops; + vcdev->vdev.type =3D VFIO_DEVICE_TYPE_CCW; + vcdev->vdev.name =3D name; + vcdev->vdev.dev =3D &vcdev->cdev.parent_obj.parent_obj; + + return 0; + +out_err: + g_free(name); + return -1; +} + static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cdev, Error **errp) { char *tmp, group_path[PATH_MAX]; @@ -327,7 +359,6 @@ static VFIOGroup *vfio_ccw_get_group(S390CCWDevice *cde= v, Error **errp) =20 static void vfio_ccw_realize(DeviceState *dev, Error **errp) { - VFIODevice *vbasedev; VFIOGroup *group; CcwDevice *ccw_dev =3D DO_UPCAST(CcwDevice, parent_obj, dev); S390CCWDevice *cdev =3D DO_UPCAST(S390CCWDevice, parent_obj, ccw_dev); @@ -348,20 +379,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error *= *errp) goto out_group_err; } =20 - vcdev->vdev.ops =3D &vfio_ccw_ops; - vcdev->vdev.type =3D VFIO_DEVICE_TYPE_CCW; - vcdev->vdev.name =3D g_strdup_printf("%x.%x.%04x", cdev->hostid.cssid, - cdev->hostid.ssid, cdev->hostid.dev= id); - vcdev->vdev.dev =3D dev; - QLIST_FOREACH(vbasedev, &group->device_list, next) { - if (strcmp(vbasedev->name, vcdev->vdev.name) =3D=3D 0) { - error_setg(&err, "vfio: subchannel %s has already been attache= d", - vcdev->vdev.name); - goto out_device_err; - } - } - - if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, &err)) { + if (vfio_ccw_get_device(group, vcdev, &err)) { goto out_device_err; } =20 @@ -380,7 +398,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **= errp) out_notifier_err: vfio_ccw_put_region(vcdev); out_region_err: - vfio_put_device(vcdev); + vfio_ccw_put_device(vcdev); out_device_err: vfio_put_group(group); out_group_err: @@ -401,7 +419,7 @@ static void vfio_ccw_unrealize(DeviceState *dev, Error = **errp) =20 vfio_ccw_unregister_io_notifier(vcdev); vfio_ccw_put_region(vcdev); - vfio_put_device(vcdev); + vfio_ccw_put_device(vcdev); vfio_put_group(group); =20 if (cdc->unrealize) {