From patchwork Fri Jul 14 14:35:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Manos Pitsidianakis X-Patchwork-Id: 9841235 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id CC5DE60212 for ; Fri, 14 Jul 2017 14:39:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id B9B5E28735 for ; Fri, 14 Jul 2017 14:39:56 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id AB3C328789; Fri, 14 Jul 2017 14:39:56 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 219A028735 for ; Fri, 14 Jul 2017 14:39:56 +0000 (UTC) Received: from localhost ([::1]:38354 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dW1l5-0006ns-FZ for patchwork-qemu-devel@patchwork.kernel.org; Fri, 14 Jul 2017 10:39:55 -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+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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, BlockDriver *drv, } else { error_setg_errno(errp, -ret, "Could not open image"); } - goto free_and_fail; + goto open_failed; } ret = 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; } bdrv_refresh_limits(bs, &local_err); if (local_err) { error_propagate(errp, local_err); - ret = -EINVAL; - goto free_and_fail; + return -EINVAL; } assert(bdrv_opt_mem_align(bs) != 0); @@ -1140,12 +1139,14 @@ static int bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, assert(is_power_of_2(bs->bl.request_alignment)); return 0; - -free_and_fail: - /* FIXME Close bs first if already opened*/ - g_free(bs->opaque); - bs->opaque = NULL; +open_failed: bs->drv = NULL; + if (bs->file != NULL) { + bdrv_unref_child(bs, bs->file); + bs->file = NULL; + } + g_free(bs->opaque); + bs->opaque = NULL; return ret; } @@ -1166,7 +1167,9 @@ BlockDriverState *bdrv_new_open_driver(BlockDriver *drv, const char *node_name, ret = bdrv_open_driver(bs, drv, node_name, bs->options, flags, errp); if (ret < 0) { QDECREF(bs->explicit_options); + bs->explicit_options = NULL; QDECREF(bs->options); + bs->options = NULL; bdrv_unref(bs); return NULL; } @@ -2600,9 +2603,6 @@ static BlockDriverState *bdrv_open_inherit(const char *filename, fail: blk_unref(file); - if (bs->file != NULL) { - bdrv_unref_child(bs, bs->file); - } QDECREF(snapshot_options); QDECREF(bs->explicit_options); QDECREF(bs->options);