From nobody Thu May 2 23:11:50 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 (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 152311236551639.60414068894738; Sat, 7 Apr 2018 07:46:05 -0700 (PDT) Received: from localhost ([::1]:32909 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4p6S-0003wL-HV for importer@patchew.org; Sat, 07 Apr 2018 10:46:04 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48472) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4p4U-0002mw-QS for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:44:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f4p4Q-0001ET-PR for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:44:02 -0400 Received: from 1.mo179.mail-out.ovh.net ([178.33.111.220]:46618) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f4p4Q-0001Du-IF for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:43:58 -0400 Received: from player737.ha.ovh.net (unknown [10.109.120.125]) by mo179.mail-out.ovh.net (Postfix) with ESMTP id 1F848B2389 for ; Sat, 7 Apr 2018 16:43:57 +0200 (CEST) Received: from [192.168.0.243] (lns-bzn-46-82-253-208-248.adsl.proxad.net [82.253.208.248]) (Authenticated sender: groug@kaod.org) by player737.ha.ovh.net (Postfix) with ESMTPA id A4045E00AF; Sat, 7 Apr 2018 16:43:52 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Sat, 07 Apr 2018 16:43:46 +0200 Message-ID: <152311222681.203086.8874800175539040298.stgit@bahia> In-Reply-To: <152311221072.203086.16767398863033055271.stgit@bahia> References: <152311221072.203086.16767398863033055271.stgit@bahia> 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: 15648882807289846071 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedtgedrhedtgdektdcutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 178.33.111.220 Subject: [Qemu-devel] [PATCH v2 1/2] vfio-ccw: fix memory leaks 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 If the subchannel is already attached or if vfio_get_device() fails, the code jumps to the 'out_device_err' label and doesn't free the string it has just allocated. The code should be reworked so that vcdev->vdev.name only gets set when the device has been attached, and freed when it is about to be detached. This could be achieved with the addition of a vfio_ccw_get_device() function that would be the counterpart of vfio_put_device(). But this is a more elaborate cleanup that should be done in a follow-up. For now, let's just add calls to g_free() on the buggy error paths. Signed-off-by: Greg Kurz --- hw/vfio/ccw.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index 4e5855741a64..fe34b507699f 100644 --- a/hw/vfio/ccw.c +++ b/hw/vfio/ccw.c @@ -357,11 +357,13 @@ static void vfio_ccw_realize(DeviceState *dev, Error = **errp) if (strcmp(vbasedev->name, vcdev->vdev.name) =3D=3D 0) { error_setg(&err, "vfio: subchannel %s has already been attache= d", vcdev->vdev.name); + g_free(vcdev->vdev.name); goto out_device_err; } } =20 if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, &err)) { + g_free(vcdev->vdev.name); goto out_device_err; } =20 From nobody Thu May 2 23:11:50 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 (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1523112509521894.8252764917022; Sat, 7 Apr 2018 07:48:29 -0700 (PDT) Received: from localhost ([::1]:33031 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4p8m-0005n7-Dj for importer@patchew.org; Sat, 07 Apr 2018 10:48:28 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48553) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1f4p4f-0002uU-DF for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:44:14 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1f4p4a-0001I9-0e for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:44:12 -0400 Received: from 3.mo177.mail-out.ovh.net ([46.105.36.172]:40388) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1f4p4Z-0001HY-PR for qemu-devel@nongnu.org; Sat, 07 Apr 2018 10:44:07 -0400 Received: from player773.ha.ovh.net (unknown [10.109.108.60]) by mo177.mail-out.ovh.net (Postfix) with ESMTP id 4E154AB895 for ; Sat, 7 Apr 2018 16:44:06 +0200 (CEST) Received: from [192.168.0.243] (lns-bzn-46-82-253-208-248.adsl.proxad.net [82.253.208.248]) (Authenticated sender: groug@kaod.org) by player773.ha.ovh.net (Postfix) with ESMTPA id 1236C600088; Sat, 7 Apr 2018 16:44:02 +0200 (CEST) From: Greg Kurz To: qemu-devel@nongnu.org Date: Sat, 07 Apr 2018 16:44:01 +0200 Message-ID: <152311224179.203086.10603697792860546407.stgit@bahia> In-Reply-To: <152311221072.203086.16767398863033055271.stgit@bahia> References: <152311221072.203086.16767398863033055271.stgit@bahia> 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: 15651697556807063863 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedtgedrhedtgdektdcutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 46.105.36.172 Subject: [Qemu-devel] [PATCH v2 2/2] vfio-ccw: introduce vfio_ccw_get_device() 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 A recent patch fixed leaks of the dynamically allocated vcdev->vdev.name field in vfio_ccw_realize(), but we now have three freeing sites for it. This is unfortunate and seems to indicate something is wrong with its life cycle. 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, including the "already attached" check, to a separate vfio_ccw_get_device() function, 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 | 56 ++++++++++++++++++++++++++++++++++++-----------------= --- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c index fe34b507699f..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,22 +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); - g_free(vcdev->vdev.name); - goto out_device_err; - } - } - - if (vfio_get_device(group, cdev->mdevid, &vcdev->vdev, &err)) { - g_free(vcdev->vdev.name); + if (vfio_ccw_get_device(group, vcdev, &err)) { goto out_device_err; } =20 @@ -382,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: @@ -403,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) {