From nobody Tue May 7 16:41:05 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org ARC-Seal: i=1; a=rsa-sha256; t=1599574193; cv=none; d=zohomail.com; s=zohoarc; b=mlEp/JXPgTF8DlyXeCpMOWAJL6i3uBrJdN6VjxOrx8+JRCd7xP6byHV4ZQyzQvBNedc4HSR2px5bCTjCkmygzbQln2n3EIyuuH6Pr9VX7vIcj/auxyK/5gH0yptC9G1BGty2TN1fOE3Jz/+bB5CKXsRCQWYZJNGJJ/tumm2tu14= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1599574193; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=u1m/mSOLdg7HxpcPkRNiNHp4pI0tAcGiOy1arYe4X8I=; b=gaNo/8YINu+eXwYosVw5gwCZ0Jhnhbc19uMeMk3vsP3KLTybzy+Z1EgKhbRo9YutmTVh7MhMXBIJ2JMs1a6SgYXm778W69dUB+lZxfctSfA7QugQCQ+s+ocxwbijQINYYVumricaEhCABJdOmunLUd4nMCV3KLAVYBGygiDmwAE= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1599574192954231.16343193859598; Tue, 8 Sep 2020 07:09:52 -0700 (PDT) Received: from localhost ([::1]:39070 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kFeJj-0003b9-0k for importer@patchew.org; Tue, 08 Sep 2020 10:09:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:32942) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFeJ2-0002dN-AH; Tue, 08 Sep 2020 10:09:08 -0400 Received: from fanzine.igalia.com ([178.60.130.6]:33999) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kFeJ0-0004GO-2r; Tue, 08 Sep 2020 10:09:08 -0400 Received: from [81.0.33.67] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1kFeId-0007AL-Gj; Tue, 08 Sep 2020 16:08:43 +0200 Received: from berto by perseus.local with local (Exim 4.92) (envelope-from ) id 1kFeIQ-0001zO-RK; Tue, 08 Sep 2020 16:08:30 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=u1m/mSOLdg7HxpcPkRNiNHp4pI0tAcGiOy1arYe4X8I=; b=kOTxDxGJ/GR3kt8PxyaEAVmCmQwcbr1P9EjzEDJyhjVkmCudZ0Gb9tY8jbyFfcH8rSdCVsjxTHRbhY9IumXStv1vSKX5YhbH4uV48HjJFi5rZbpf970ATssFMIsYmS0ZKjz+W5kUGPPlC2ALLTq6vi3phJ9zRLawcQugBLdlaek6bdSKmGnZ7hDrlHl3VCk0MGk6adTASG/mJ5vDuaXIoC4wzf6PSh+BJUmsukTPGfIQvOFZI4mWpjt38Ws6oIxblncI3SaQutAt31tzOmIFwO5cmRP3MgMriIkIFd+mHLUpk2BMBNZXcMD/5OzZMhS90lmIeq3RDZvezNqg07QmyQ==; From: Alberto Garcia To: qemu-devel@nongnu.org Subject: [PATCH 1/2] qcow2: Handle QCowL2Meta on error in preallocate_co() Date: Tue, 8 Sep 2020 16:08:27 +0200 Message-Id: X-Mailer: git-send-email 2.20.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=178.60.130.6; envelope-from=berto@igalia.com; helo=fanzine.igalia.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/08 10:08:43 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" If qcow2_alloc_cluster_offset() or qcow2_alloc_cluster_link_l2() fail then this function simply returns the error code, potentially leaking the QCowL2Meta structure and leaving stale items in s->cluster_allocs. A second problem is that this function calls qcow2_free_any_clusters() on failure but passing a host cluster offset instead of an L2 entry. Luckily for normal uncompressed clusters a raw offset also works like a valid L2 entry so it works just the same, but we should be using qcow2_free_clusters() instead. This patch fixes both problems by using qcow2_handle_l2meta(). Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf --- block/qcow2.c | 40 +++++++++++++++++----------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/block/qcow2.c b/block/qcow2.c index da56b1a4df..eeb125c697 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -2102,7 +2102,6 @@ static coroutine_fn int qcow2_handle_l2meta(BlockDriv= erState *bs, QCowL2Meta *next; =20 if (link_l2) { - assert(!l2meta->prealloc); ret =3D qcow2_alloc_cluster_link_l2(bs, l2meta); if (ret) { goto out; @@ -3126,7 +3125,7 @@ static int coroutine_fn preallocate_co(BlockDriverSta= te *bs, uint64_t offset, int64_t file_length; unsigned int cur_bytes; int ret; - QCowL2Meta *meta; + QCowL2Meta *meta =3D NULL, *m; =20 assert(offset <=3D new_length); bytes =3D new_length - offset; @@ -3137,27 +3136,17 @@ static int coroutine_fn preallocate_co(BlockDriverS= tate *bs, uint64_t offset, &host_offset, &meta); if (ret < 0) { error_setg_errno(errp, -ret, "Allocating clusters failed"); - return ret; + goto out; } =20 - while (meta) { - QCowL2Meta *next =3D meta->next; - meta->prealloc =3D true; - - ret =3D qcow2_alloc_cluster_link_l2(bs, meta); - if (ret < 0) { - error_setg_errno(errp, -ret, "Mapping clusters failed"); - qcow2_free_any_clusters(bs, meta->alloc_offset, - meta->nb_clusters, QCOW2_DISCARD_N= EVER); - return ret; - } - - /* There are no dependent requests, but we need to remove our - * request from the list of in-flight requests */ - QLIST_REMOVE(meta, next_in_flight); + for (m =3D meta; m !=3D NULL; m =3D m->next) { + m->prealloc =3D true; + } =20 - g_free(meta); - meta =3D next; + ret =3D qcow2_handle_l2meta(bs, &meta, true); + if (ret < 0) { + error_setg_errno(errp, -ret, "Mapping clusters failed"); + goto out; } =20 /* TODO Preallocate data if requested */ @@ -3174,7 +3163,8 @@ static int coroutine_fn preallocate_co(BlockDriverSta= te *bs, uint64_t offset, file_length =3D bdrv_getlength(s->data_file->bs); if (file_length < 0) { error_setg_errno(errp, -file_length, "Could not get file size"); - return file_length; + ret =3D file_length; + goto out; } =20 if (host_offset + cur_bytes > file_length) { @@ -3184,11 +3174,15 @@ static int coroutine_fn preallocate_co(BlockDriverS= tate *bs, uint64_t offset, ret =3D bdrv_co_truncate(s->data_file, host_offset + cur_bytes, fa= lse, mode, 0, errp); if (ret < 0) { - return ret; + goto out; } } =20 - return 0; + ret =3D 0; + +out: + qcow2_handle_l2meta(bs, &meta, false); + return ret; } =20 /* qcow2_refcount_metadata_size: --=20 2.20.1 From nobody Tue May 7 16:41:05 2024 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org ARC-Seal: i=1; a=rsa-sha256; t=1599574193; cv=none; d=zohomail.com; s=zohoarc; b=mPu3Kc3lAPvGUIC82jgeLbY7b0Tacicg2/rb7Mdbb038EaoXmRTM/BUpOsR2/5Hq7Tb+xMzeCKpYbesYza/9u11RC80ycy4/mFEn3HOvAQAoG9nbC2JRjMAMUWdikaXW8AeS0jR070C3k9MmAkWFGqF//VBDaA0w8oSXlWwVWIk= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1599574193; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=iURH801BBUb7sWphpIXEwbs0fpz4AS7JQPtbXPlcCuQ=; b=AUPKGa5m93OVzlIvzGWiL7MT5FBSks9rQbDrr2pWu2q7S5K3QW4OTJlzRXtWp0oCAQ3nB6KDR2haxrNaMjnhma16k9gNJ0gj02mAOlm15nxRtVCF3Rr39m3w1SGoH0yNYlavb5ILnEug+rVAs1ewVVIM/L4yDmP60W5HTqZC6Rk= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 15995741928970.5372100660583783; Tue, 8 Sep 2020 07:09:52 -0700 (PDT) Received: from localhost ([::1]:39100 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1kFeJj-0003bu-4k for importer@patchew.org; Tue, 08 Sep 2020 10:09:51 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:32958) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1kFeJ3-0002dW-Gn; Tue, 08 Sep 2020 10:09:09 -0400 Received: from fanzine.igalia.com ([178.60.130.6]:34004) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1kFeJ0-0004GP-2a; Tue, 08 Sep 2020 10:09:09 -0400 Received: from [81.0.33.67] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1kFeIe-0007AN-3R; Tue, 08 Sep 2020 16:08:44 +0200 Received: from berto by perseus.local with local (Exim 4.92) (envelope-from ) id 1kFeIR-0001zR-9q; Tue, 08 Sep 2020 16:08:31 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:MIME-Version:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=iURH801BBUb7sWphpIXEwbs0fpz4AS7JQPtbXPlcCuQ=; b=ADxM9t4/x5czklqh4/H94oCMbyss7RRYmrzXMlZLQ8F6Wyt33eAGzeT2wD1KA18CsnJp+eHurnZPp3Mo4d6eZKAplszjsyhrXiGVmVGSLGArzR+vdTrL+SAE8dpSLsaMVnBkOfcVNVaebXxd/Ks/nQ0wBy02SkeHaHT5flqq76VOg4ZXTioPinHsr73+St5TpBh+VYeo5e2xQB8F5O5vHDuNylSyhRYz/+VcrrWVcSK17n4TR3oU3Tcqff7neevQiHpFIPvqv3x22FYPu64SgURnqItrHUZSbMbVDlpYDxH92t8F9WSUcoQLnzW/UQ7HGgkBRTmXgRGMouGwWuoGhw==; From: Alberto Garcia To: qemu-devel@nongnu.org Subject: [PATCH 2/2] qcow2: Make qcow2_free_any_clusters() free only one cluster Date: Tue, 8 Sep 2020 16:08:28 +0200 Message-Id: <77cea0f4616f921d37e971b3c5b18a2faa24b173.1599573989.git.berto@igalia.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Received-SPF: pass client-ip=178.60.130.6; envelope-from=berto@igalia.com; helo=fanzine.igalia.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/09/08 10:08:43 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Alberto Garcia , qemu-block@nongnu.org, Max Reitz Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" This function takes an L2 entry and a number of clusters to free. Although in principle it can free any type of cluster (using the L2 entry to determine its type) in practice the API is broken because compressed clusters have a variable size and there is no way to free more than one without having the L2 entry of each one of them. The good news all callers are passing nb_clusters=3D1 so we can simply get rid of that parameter. Signed-off-by: Alberto Garcia Reviewed-by: Kevin Wolf --- block/qcow2.h | 4 ++-- block/qcow2-cluster.c | 6 +++--- block/qcow2-refcount.c | 8 ++++---- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/block/qcow2.h b/block/qcow2.h index 065ec3df0b..bb6358121d 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -855,8 +855,8 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int siz= e); void qcow2_free_clusters(BlockDriverState *bs, int64_t offset, int64_t size, enum qcow2_discard_type type); -void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry, - int nb_clusters, enum qcow2_discard_type type= ); +void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry, + enum qcow2_discard_type type); =20 int qcow2_update_snapshot_refcount(BlockDriverState *bs, int64_t l1_table_offset, int l1_size, int addend); diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index 996b3314f4..89c561e4c0 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1096,7 +1096,7 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs,= QCowL2Meta *m) */ if (!m->keep_old_clusters && j !=3D 0) { for (i =3D 0; i < j; i++) { - qcow2_free_any_clusters(bs, old_cluster[i], 1, QCOW2_DISCARD_N= EVER); + qcow2_free_any_cluster(bs, old_cluster[i], QCOW2_DISCARD_NEVER= ); } } =20 @@ -1911,7 +1911,7 @@ static int discard_in_l2_slice(BlockDriverState *bs, = uint64_t offset, set_l2_bitmap(s, l2_slice, l2_index + i, new_l2_bitmap); } /* Then decrease the refcount */ - qcow2_free_any_clusters(bs, old_l2_entry, 1, type); + qcow2_free_any_cluster(bs, old_l2_entry, type); } =20 qcow2_cache_put(s->l2_table_cache, (void **) &l2_slice); @@ -2003,7 +2003,7 @@ static int zero_in_l2_slice(BlockDriverState *bs, uin= t64_t offset, =20 qcow2_cache_entry_mark_dirty(s->l2_table_cache, l2_slice); if (unmap) { - qcow2_free_any_clusters(bs, old_l2_entry, 1, QCOW2_DISCARD_REQ= UEST); + qcow2_free_any_cluster(bs, old_l2_entry, QCOW2_DISCARD_REQUEST= ); } set_l2_entry(s, l2_slice, l2_index + i, new_l2_entry); if (has_subclusters(s)) { diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c index aae52607eb..fc9bb2258f 100644 --- a/block/qcow2-refcount.c +++ b/block/qcow2-refcount.c @@ -1156,8 +1156,8 @@ void qcow2_free_clusters(BlockDriverState *bs, * Free a cluster using its L2 entry (handles clusters of all types, e.g. * normal cluster, compressed cluster, etc.) */ -void qcow2_free_any_clusters(BlockDriverState *bs, uint64_t l2_entry, - int nb_clusters, enum qcow2_discard_type type) +void qcow2_free_any_cluster(BlockDriverState *bs, uint64_t l2_entry, + enum qcow2_discard_type type) { BDRVQcow2State *s =3D bs->opaque; QCow2ClusterType ctype =3D qcow2_get_cluster_type(bs, l2_entry); @@ -1168,7 +1168,7 @@ void qcow2_free_any_clusters(BlockDriverState *bs, ui= nt64_t l2_entry, ctype =3D=3D QCOW2_CLUSTER_ZERO_ALLOC)) { bdrv_pdiscard(s->data_file, l2_entry & L2E_OFFSET_MASK, - nb_clusters << s->cluster_bits); + s->cluster_size); } return; } @@ -1191,7 +1191,7 @@ void qcow2_free_any_clusters(BlockDriverState *bs, ui= nt64_t l2_entry, l2_entry & L2E_OFFSET_MASK); } else { qcow2_free_clusters(bs, l2_entry & L2E_OFFSET_MASK, - nb_clusters << s->cluster_bits, type); + s->cluster_size, type); } break; case QCOW2_CLUSTER_ZERO_PLAIN: --=20 2.20.1