From nobody Mon Feb 9 00:42:29 2026 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 1510346044447280.10477809724864; Fri, 10 Nov 2017 12:34:04 -0800 (PST) Received: from localhost ([::1]:43389 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eDFzu-0002rn-Af for importer@patchew.org; Fri, 10 Nov 2017 15:33:54 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54982) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eDFxr-0001Vp-GA for qemu-devel@nongnu.org; Fri, 10 Nov 2017 15:31:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eDFxq-0004Kg-De for qemu-devel@nongnu.org; Fri, 10 Nov 2017 15:31:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47768) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eDFxl-0004Hj-F1; Fri, 10 Nov 2017 15:31:41 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9B8AB7EAB1; Fri, 10 Nov 2017 20:31:40 +0000 (UTC) Received: from localhost (ovpn-204-126.brq.redhat.com [10.40.204.126]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 23B3662669; Fri, 10 Nov 2017 20:31:39 +0000 (UTC) From: Max Reitz To: qemu-block@nongnu.org Date: Fri, 10 Nov 2017 21:31:11 +0100 Message-Id: <20171110203111.7666-6-mreitz@redhat.com> In-Reply-To: <20171110203111.7666-1-mreitz@redhat.com> References: <20171110203111.7666-1-mreitz@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 10 Nov 2017 20:31:40 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache 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 , John Snow , Alberto Garcia , qemu-devel@nongnu.org, 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" Instead of using an assertion, it is better to emit a corruption event here. Checking all offsets for correct alignment can be tedious and it is easily possible to forget to do so. qcow2_cache_do_get() is a function every L2 and refblock access has to go through, so this is a good central point to add such a check. And for good measure, let us also add an assertion that the offset is non-zero. Making this a corruption event is not feasible, because a zero offset usually means something special (such as the cluster is unused), so all callers should be checking this anyway. If they do not, it is their fault, hence the assertion here. Signed-off-by: Max Reitz Reviewed-by: Alberto Garcia Reviewed-by: Eric Blake --- block/qcow2-cache.c | 21 +++++++++++++++++++++ tests/qemu-iotests/060 | 21 +++++++++++++++++++++ tests/qemu-iotests/060.out | 29 +++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+) diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c index 75746a7f43..a5baaba0ff 100644 --- a/block/qcow2-cache.c +++ b/block/qcow2-cache.c @@ -62,6 +62,18 @@ static inline int qcow2_cache_get_table_idx(BlockDriverS= tate *bs, return idx; } =20 +static inline const char *qcow2_cache_get_name(BDRVQcow2State *s, Qcow2Cac= he *c) +{ + if (c =3D=3D s->refcount_block_cache) { + return "refcount block"; + } else if (c =3D=3D s->l2_table_cache) { + return "L2 table"; + } else { + /* Do not abort, because this is not critical */ + return "unknown"; + } +} + static void qcow2_cache_table_release(BlockDriverState *bs, Qcow2Cache *c, int i, int num_tables) { @@ -314,9 +326,18 @@ static int qcow2_cache_do_get(BlockDriverState *bs, Qc= ow2Cache *c, uint64_t min_lru_counter =3D UINT64_MAX; int min_lru_index =3D -1; =20 + assert(offset !=3D 0); + trace_qcow2_cache_get(qemu_coroutine_self(), c =3D=3D s->l2_table_cach= e, offset, read_from_disk); =20 + if (offset_into_cluster(s, offset)) { + qcow2_signal_corruption(bs, true, -1, -1, "Cannot get entry from %= s " + "cache: Offset %#" PRIx64 " is unaligned", + qcow2_cache_get_name(s, c), offset); + return -EIO; + } + /* Check if the table is already cached */ i =3D lookup_index =3D (offset / s->cluster_size * 4) % c->size; do { diff --git a/tests/qemu-iotests/060 b/tests/qemu-iotests/060 index c230696b3a..1eca09417b 100755 --- a/tests/qemu-iotests/060 +++ b/tests/qemu-iotests/060 @@ -405,6 +405,27 @@ _check_test_img -r all $QEMU_IMG resize --shrink "$TEST_IMG" 32M _img_info | grep 'virtual size' =20 +echo +echo "=3D=3D=3D Discarding a refblock covered by an unaligned refblock =3D= =3D=3D" +echo + +IMGOPTS=3D'refcount_bits=3D1' _make_test_img 64M + +# Same as above +poke_file "$TEST_IMG" "$(($rt_offset+8))" "\x00\x00\x00\x10\x00\x00\x00\x0= 0" +# But now we actually "create" an unaligned third refblock +poke_file "$TEST_IMG" "$(($rt_offset+16))" "\x00\x00\x00\x00\x00\x00\x02\x= 00" +$QEMU_IMG resize --shrink "$TEST_IMG" 32M + +echo '--- Repairing ---' +# Fails the first repair because the corruption prevents the check +# function from double-checking +# (Using -q for the first invocation, because otherwise the +# double-check error message appears above the summary for some +# reason -- so let's just hide the summary) +_check_test_img -q -r all +_check_test_img -r all + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/060.out b/tests/qemu-iotests/060.out index 358e54cdc9..56f5eb15d8 100644 --- a/tests/qemu-iotests/060.out +++ b/tests/qemu-iotests/060.out @@ -370,4 +370,33 @@ virtual size: 64M (67108864 bytes) No errors were found on the image. Image resized. virtual size: 32M (33554432 bytes) + +=3D=3D=3D Discarding a refblock covered by an unaligned refblock =3D=3D=3D + +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 +qcow2: Marking image as corrupt: Cannot get entry from refcount block cach= e: Offset 0x200 is unaligned; further corruption events will be suppressed +qemu-img: Failed to discard unused refblocks: Input/output error +--- Repairing --- +Repairing refcount block 1 is outside image +ERROR refcount block 2 is not cluster aligned; refcount table entry corrup= ted +qcow2: Marking image as corrupt: Refblock offset 0x200 unaligned (reftable= index: 0x2); further corruption events will be suppressed +Can't get refcount for cluster 1048576: Input/output error +Rebuilding refcount structure +Repairing cluster 1 refcount=3D1 reference=3D0 +Repairing cluster 2 refcount=3D1 reference=3D0 +Repairing cluster 1048576 refcount=3D1 reference=3D0 +qemu-img: Check failed: No medium found +Leaked cluster 1 refcount=3D1 reference=3D0 +Leaked cluster 2 refcount=3D1 reference=3D0 +Leaked cluster 1048576 refcount=3D1 reference=3D0 +Repairing cluster 1 refcount=3D1 reference=3D0 +Repairing cluster 2 refcount=3D1 reference=3D0 +Repairing cluster 1048576 refcount=3D1 reference=3D0 +The following inconsistencies were found and repaired: + + 3 leaked clusters + 0 corruptions + +Double checking the fixed image now... +No errors were found on the image. *** done --=20 2.13.6