From nobody Tue Apr 16 09:04:32 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 1500043124392271.1696460619073; Fri, 14 Jul 2017 07:38:44 -0700 (PDT) Received: from localhost ([::1]:38349 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1jv-0005Vv-6g for importer@patchew.org; Fri, 14 Jul 2017 10:38:43 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53541) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1hb-0003zk-9d for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dW1hY-0003Q1-Ck for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:19 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:16577) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1hY-0003PI-1c for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:16 -0400 Received: from mail.ntua.gr (ppp046176127036.access.hol.gr [46.176.127.36]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v6EEaDRL065278 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Jul 2017 17:36:13 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host ppp046176127036.access.hol.gr [46.176.127.36] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 14 Jul 2017 17:35:47 +0300 Message-Id: <20170714143548.32559-2-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170714143548.32559-1-el13635@mail.ntua.gr> References: <20170714143548.32559-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 1/2] block: fix dangling bs->explicit_options in block.c 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" In some error paths it is possible to QDECREF a freed dangling explicit_options, resulting in a heap overflow crash. For example bdrv_open_inherit()'s fail unrefs it, then calls bdrv_unref which calls bdrv_close which also unrefs it. Signed-off-by: Manos Pitsidianakis Reviewed-by: Eric Blake --- block.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/block.c b/block.c index 98a9209371..96b912d229 100644 --- a/block.c +++ b/block.c @@ -2608,6 +2608,7 @@ fail: QDECREF(bs->options); QDECREF(options); bs->options =3D NULL; + bs->explicit_options =3D NULL; bdrv_unref(bs); error_propagate(errp, local_err); return NULL; @@ -3087,6 +3088,7 @@ static void bdrv_close(BlockDriverState *bs) QDECREF(bs->options); QDECREF(bs->explicit_options); bs->options =3D NULL; + bs->explicit_options =3D NULL; QDECREF(bs->full_open_options); bs->full_open_options =3D NULL; } --=20 2.11.0 From nobody Tue Apr 16 09:04:32 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 1500043210369672.733782644374; Fri, 14 Jul 2017 07:40:10 -0700 (PDT) Received: from localhost ([::1]:38355 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1lG-0007Yp-V6 for importer@patchew.org; Fri, 14 Jul 2017 10:40:06 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1he-00042g-E8 for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dW1hd-0003SA-FW for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:22 -0400 Received: from smtp1.ntua.gr ([2001:648:2000:de::183]:16586) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1hd-0003RK-3a for qemu-devel@nongnu.org; Fri, 14 Jul 2017 10:36:21 -0400 Received: from mail.ntua.gr (ppp046176127036.access.hol.gr [46.176.127.36]) (authenticated bits=0) by smtp1.ntua.gr (8.15.2/8.15.2) with ESMTPSA id v6EEaIn7065347 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 14 Jul 2017 17:36:18 +0300 (EEST) (envelope-from el13635@mail.ntua.gr) X-Authentication-Warning: smtp1.ntua.gr: Host ppp046176127036.access.hol.gr [46.176.127.36] claimed to be mail.ntua.gr From: Manos Pitsidianakis To: qemu-devel Date: Fri, 14 Jul 2017 17:35:48 +0300 Message-Id: <20170714143548.32559-3-el13635@mail.ntua.gr> X-Mailer: git-send-email 2.11.0 In-Reply-To: <20170714143548.32559-1-el13635@mail.ntua.gr> References: <20170714143548.32559-1-el13635@mail.ntua.gr> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2001:648:2000:de::183 Subject: [Qemu-devel] [PATCH v3 2/2] block: fix leaks in bdrv_open_driver() 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: Kevin Wolf , Alberto Garcia , Stefan Hajnoczi , qemu-block , Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" bdrv_open_driver() is called in two places, bdrv_new_open_driver() and bdrv_open_common(). In the latter, failure cleanup in is in its caller, bdrv_open_inherit(), which unrefs the bs->file of the failed driver open if it exists. Let's move the bs->file cleanup to bdrv_open_driver() to take care of all callers and do not set bs->drv to NULL unless the driver's open function failed. When bs is destroyed by removing its last reference, it calls bdrv_close() which checks bs->drv to perform the needed cleanups and also call the driver's close function. Since it cleans up options and opaque we must take care not leave dangling pointers. The error paths in bdrv_open_driver() are now two: If open fails, drv->bdrv_close() should not be called. Unref the child if it exists, free what we allocated and set bs->drv to NULL. Return the error and let callers free their stuff. If open succeeds but we fail after, return the error and let callers unref and delete their bs, while cleaning up their allocations. Signed-off-by: Manos Pitsidianakis --- block.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/block.c b/block.c index 96b912d229..9ff62439be 100644 --- a/block.c +++ b/block.c @@ -1119,20 +1119,19 @@ static int bdrv_open_driver(BlockDriverState *bs, B= lockDriver *drv, } else { error_setg_errno(errp, -ret, "Could not open image"); } - goto free_and_fail; + goto open_failed; } =20 ret =3D refresh_total_sectors(bs, bs->total_sectors); if (ret < 0) { error_setg_errno(errp, -ret, "Could not refresh total sector count= "); - goto free_and_fail; + return ret; } =20 bdrv_refresh_limits(bs, &local_err); if (local_err) { error_propagate(errp, local_err); - ret =3D -EINVAL; - goto free_and_fail; + return -EINVAL; } =20 assert(bdrv_opt_mem_align(bs) !=3D 0); @@ -1140,12 +1139,14 @@ static int bdrv_open_driver(BlockDriverState *bs, B= lockDriver *drv, assert(is_power_of_2(bs->bl.request_alignment)); =20 return 0; - -free_and_fail: - /* FIXME Close bs first if already opened*/ - g_free(bs->opaque); - bs->opaque =3D NULL; +open_failed: bs->drv =3D NULL; + if (bs->file !=3D NULL) { + bdrv_unref_child(bs, bs->file); + bs->file =3D NULL; + } + g_free(bs->opaque); + bs->opaque =3D NULL; return ret; } =20 @@ -1166,7 +1167,9 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *d= rv, const char *node_name, ret =3D bdrv_open_driver(bs, drv, node_name, bs->options, flags, errp); if (ret < 0) { QDECREF(bs->explicit_options); + bs->explicit_options =3D NULL; QDECREF(bs->options); + bs->options =3D NULL; bdrv_unref(bs); return NULL; } @@ -2600,9 +2603,6 @@ static BlockDriverState *bdrv_open_inherit(const char= *filename, =20 fail: blk_unref(file); - if (bs->file !=3D NULL) { - bdrv_unref_child(bs, bs->file); - } QDECREF(snapshot_options); QDECREF(bs->explicit_options); QDECREF(bs->options); --=20 2.11.0