From nobody Wed Oct 22 15:02:26 2025 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; dkim=fail; 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 1518712393298725.8728990711696; Thu, 15 Feb 2018 08:33:13 -0800 (PST) Received: from localhost ([::1]:36979 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emMT7-0003D3-Dt for importer@patchew.org; Thu, 15 Feb 2018 11:33:09 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53419) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emMRR-0002Oo-3O for qemu-devel@nongnu.org; Thu, 15 Feb 2018 11:31:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1emMRN-0007Xk-3P for qemu-devel@nongnu.org; Thu, 15 Feb 2018 11:31:25 -0500 Received: from fanzine.igalia.com ([91.117.99.155]:37360) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1emMRM-0007Wj-LU; Thu, 15 Feb 2018 11:31:21 -0500 Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1emMRJ-0004cE-Uv; Thu, 15 Feb 2018 17:31:18 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1emMR1-0007vY-PV; Thu, 15 Feb 2018 18:30:59 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=xcxwCWZN4u+l5lxQlYaqgYBhV02ALhxxNJBMd6nubPE=; b=GLoAPeqYgwvAIdtSue28xWQfh0zfiRhiDUQUgZV5uqPOt/VrZFY/SsWvHzU8pjRzo+rffS7E3IxOuEWGAS3xJmJm+Z1J9bvLDXXTPhd+d25Th410a2OBJEVso2fkZBiyNobxd0vjWI8GxIsH2WsakEbQlKPugTRQyWFaRjznzI5MQyfi+K8DFYm1m1ywU0JiiAnrh0Qc5gBgZd8YaoJA2PTkItbwYGL3cYCZAZ3manJ7sZA0tV1qNQp0iVvHiX+9iE4s1U7c4eqduXFzcdWQFMppazbL0VUI2LsEWsRfHJGlVZyoCS32vT4bphuBdLuswIqLskpmxZMlD81ld2G1vA==; From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 15 Feb 2018 18:30:54 +0200 Message-Id: <894433c083e20bbb82e208ea8b067b13609dbc19.1518711805.git.berto@igalia.com> X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [RFC PATCH 1/2] qcow2: Allow checking and repairing corrupted internal snapshots 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 , 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) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" The L1 table parameters of internal snapshots are generally not checked by QEMU. This patch allows 'qemu-img check' to detect broken snapshots and to skip them when doing the refcount consistency check. Since without an L1 table we don't have a reliable way to recover the data from the snapshot, when 'qemu-img check' runs in repair mode this patch simply removes the corrupted snapshots. Signed-off-by: Alberto Garcia --- block/qcow2-snapshot.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++= ++++ block/qcow2.c | 7 ++++++- block/qcow2.h | 2 ++ 3 files changed, 61 insertions(+), 1 deletion(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index cee25f582b..7a36073e3e 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -736,3 +736,56 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, =20 return 0; } + +/* Check the snapshot table and optionally delete the corrupted entries */ +int qcow2_snapshot_table_check(BlockDriverState *bs, BdrvCheckResult *resu= lt, + BdrvCheckMode fix) +{ + BDRVQcow2State *s =3D bs->opaque; + bool keep_checking; + int ret, i; + + do { + keep_checking =3D false; + + for (i =3D 0; i < s->nb_snapshots; i++) { + QCowSnapshot *sn =3D s->snapshots + i; + bool found_corruption =3D false; + + if (offset_into_cluster(s, sn->l1_table_offset)) { + fprintf(stderr, "%s snapshot %s (%s) l1_offset=3D%#" PRIx6= 4 ": " + "L1 table is not cluster aligned; snapshot table e= ntry " + "corrupted\n", + (fix & BDRV_FIX_ERRORS) ? "Deleting" : "ERROR", + sn->id_str, sn->name, sn->l1_table_offset); + found_corruption =3D true; + } else if (sn->l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) { + fprintf(stderr, "%s snapshot %s (%s) l1_size=3D%#" PRIx32 = ": " + "L1 table is too large; snapshot table entry corru= pted\n", + (fix & BDRV_FIX_ERRORS) ? "Deleting" : "ERROR", + sn->id_str, sn->name, sn->l1_size); + found_corruption =3D true; + } + + if (found_corruption) { + result->corruptions++; + sn->l1_size =3D 0; /* Prevent this L1 table from being use= d */ + if (fix & BDRV_FIX_ERRORS) { + ret =3D qcow2_snapshot_delete(bs, sn->id_str, sn->name= , NULL); + if (ret < 0) { + return ret; + } + result->corruptions_fixed++; + /* If we modified the snapshot table we can't keep + * iterating. We have to start again from the + * beginning instead. */ + keep_checking =3D true; + break; + } + } + } + + } while (keep_checking); + + return 0; +} diff --git a/block/qcow2.c b/block/qcow2.c index 2c6c33b67c..20e16ea602 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -546,7 +546,12 @@ int qcow2_mark_consistent(BlockDriverState *bs) static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix) { - int ret =3D qcow2_check_refcounts(bs, result, fix); + int ret =3D qcow2_snapshot_table_check(bs, result, fix); + if (ret < 0) { + return ret; + } + + ret =3D qcow2_check_refcounts(bs, result, fix); if (ret < 0) { return ret; } diff --git a/block/qcow2.h b/block/qcow2.h index 1a84cc77b0..19f14bfc1e 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -637,6 +637,8 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, const char *snapshot_id, const char *name, Error **errp); +int qcow2_snapshot_table_check(BlockDriverState *bs, BdrvCheckResult *resu= lt, + BdrvCheckMode fix); =20 void qcow2_free_snapshots(BlockDriverState *bs); int qcow2_read_snapshots(BlockDriverState *bs); --=20 2.11.0 From nobody Wed Oct 22 15:02:26 2025 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; dkim=fail; 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 1518712508128829.3928271728148; Thu, 15 Feb 2018 08:35:08 -0800 (PST) Received: from localhost ([::1]:37152 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emMV1-0004ml-A4 for importer@patchew.org; Thu, 15 Feb 2018 11:35:07 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53450) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1emMRT-0002P6-9q for qemu-devel@nongnu.org; Thu, 15 Feb 2018 11:31:35 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1emMRN-0007Xc-2d for qemu-devel@nongnu.org; Thu, 15 Feb 2018 11:31:27 -0500 Received: from fanzine.igalia.com ([91.117.99.155]:37359) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1emMRM-0007Wk-LX; Thu, 15 Feb 2018 11:31:21 -0500 Received: from [194.100.51.2] (helo=perseus.local) by fanzine.igalia.com with esmtpsa (Cipher TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim) id 1emMRJ-0004cI-Um; Thu, 15 Feb 2018 17:31:18 +0100 Received: from berto by perseus.local with local (Exim 4.89) (envelope-from ) id 1emMR1-0007va-Qa; Thu, 15 Feb 2018 18:30:59 +0200 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=References:In-Reply-To:References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From; bh=+lj4tqDIt43KSRKug7ZG0zAFHGedNNT66D9e1/8WbWk=; b=BEwtbBai6RjBXxoEq7ELkatTZ2hWogUXLbmGgQnLuKE7KjTf8bDXJ6SMb8HFduErCvO9yNVqrzNk2RQjHOgQif/vSIfY7z0v+v9gp/l+n4IlPMpkeBeEtEwwnsCsmaPOMpTrsIkd77BvN4Q2zBcAV7ZYvtHLWzRgeHEYmSXMv8xmG4gqb5zQ+phk3V6wDbd3sd2zjRCfXeUg/ugzDZggT5nrA2xS3hy2Aj6T3LE2BDD+8EqkJo4QcsBIVp9ym3gAykL6/SJwP3k5NxZ2kwTf9GKRGUDR2RPmO0/oKbgSL3Ld2BTWEHTzXHaTXpajksYbOjwI1OOm26aA+6/ATwbsMQ==; From: Alberto Garcia To: qemu-devel@nongnu.org Date: Thu, 15 Feb 2018 18:30:55 +0200 Message-Id: X-Mailer: git-send-email 2.11.0 In-Reply-To: References: In-Reply-To: References: X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] [fuzzy] X-Received-From: 91.117.99.155 Subject: [Qemu-devel] [RFC PATCH 2/2] qcow2: Check the L1 table parameters from all internal snapshots 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 , 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) X-ZohoMail: RDKM_2 RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" The code that reads the qcow2 snapshot table takes the offset and size of all snapshots' L1 table without doing any kind of checks. Although qcow2_snapshot_load_tmp() does verify that the table size is valid, the table offset is not checked at all. On top of that there are several other code paths that deal with internal snapshots that don't use that function or do any other check. This patch verifies that all L1 tables are correctly aligned and the size does not exceed the maximum allowed valued. The check from qcow2_snapshot_load_tmp() is removed since it's now useless (opening an image will fail before that). Signed-off-by: Alberto Garcia --- block/qcow2-snapshot.c | 18 +++++++++++++----- block/qcow2.c | 2 +- block/qcow2.h | 2 +- tests/qemu-iotests/080 | 16 +++++++++++++++- tests/qemu-iotests/080.out | 42 ++++++++++++++++++++++++++++++++++++++++-- 5 files changed, 70 insertions(+), 10 deletions(-) diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c index 7a36073e3e..4424b7f1dc 100644 --- a/block/qcow2-snapshot.c +++ b/block/qcow2-snapshot.c @@ -44,7 +44,7 @@ void qcow2_free_snapshots(BlockDriverState *bs) s->nb_snapshots =3D 0; } =20 -int qcow2_read_snapshots(BlockDriverState *bs) +int qcow2_read_snapshots(BlockDriverState *bs, int flags) { BDRVQcow2State *s =3D bs->opaque; QCowSnapshotHeader h; @@ -121,6 +121,18 @@ int qcow2_read_snapshots(BlockDriverState *bs) offset +=3D name_size; sn->name[name_size] =3D '\0'; =20 + if (!(flags & BDRV_O_CHECK)) { + if (offset_into_cluster(s, sn->l1_table_offset)) { + ret =3D -EINVAL; + goto fail; + } + + if (sn->l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) { + ret =3D -EFBIG; + goto fail; + } + } + if (offset - s->snapshots_offset > QCOW_MAX_SNAPSHOTS_SIZE) { ret =3D -EFBIG; goto fail; @@ -704,10 +716,6 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs, sn =3D &s->snapshots[snapshot_index]; =20 /* Allocate and read in the snapshot's L1 table */ - if (sn->l1_size > QCOW_MAX_L1_SIZE / sizeof(uint64_t)) { - error_setg(errp, "Snapshot L1 table too large"); - return -EFBIG; - } new_l1_bytes =3D sn->l1_size * sizeof(uint64_t); new_l1_table =3D qemu_try_blockalign(bs->file->bs, ROUND_UP(new_l1_bytes, 512)); diff --git a/block/qcow2.c b/block/qcow2.c index 20e16ea602..f56947aebf 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1474,7 +1474,7 @@ static int qcow2_do_open(BlockDriverState *bs, QDict = *options, int flags, s->snapshots_offset =3D header.snapshots_offset; s->nb_snapshots =3D header.nb_snapshots; =20 - ret =3D qcow2_read_snapshots(bs); + ret =3D qcow2_read_snapshots(bs, flags); if (ret < 0) { error_setg_errno(errp, -ret, "Could not read snapshots"); goto fail; diff --git a/block/qcow2.h b/block/qcow2.h index 19f14bfc1e..fcd336aec4 100644 --- a/block/qcow2.h +++ b/block/qcow2.h @@ -641,7 +641,7 @@ int qcow2_snapshot_table_check(BlockDriverState *bs, Bd= rvCheckResult *result, BdrvCheckMode fix); =20 void qcow2_free_snapshots(BlockDriverState *bs); -int qcow2_read_snapshots(BlockDriverState *bs); +int qcow2_read_snapshots(BlockDriverState *bs, int flags); =20 /* qcow2-cache.c functions */ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables, diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080 index 1c2bd85742..728a5f937a 100755 --- a/tests/qemu-iotests/080 +++ b/tests/qemu-iotests/080 @@ -171,13 +171,27 @@ poke_file "$TEST_IMG" "$offset_l2_table_0" "\x80\x00\= x00\xff\xff\xff\x00\x00" { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter= _testdir =20 echo -echo "=3D=3D Invalid snapshot L1 table =3D=3D" +echo "=3D=3D Invalid snapshot L1 table offset =3D=3D" +_make_test_img 64M +{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_= testdir +{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter= _testdir +poke_file "$TEST_IMG" "$offset_snap1_l1_offset" "\x00\x00\x00\x00\x00\x40\= x02\x00" +{ $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_tes= tdir + +# Fix the image +_check_test_img -r all + +echo +echo "=3D=3D Invalid snapshot L1 table size =3D=3D" _make_test_img 64M { $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_= testdir { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir poke_file "$TEST_IMG" "$offset_snap1_l1_size" "\x10\x00\x00\x00" { $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_tes= tdir =20 +# Fix the image +_check_test_img -r all + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out index 6a7fda1356..eb26496566 100644 --- a/tests/qemu-iotests/080.out +++ b/tests/qemu-iotests/080.out @@ -59,9 +59,47 @@ wrote 512/512 bytes at offset 0 qemu-img: Could not create snapshot 'test': -27 (File too large) qemu-img: Could not create snapshot 'test': -11 (Resource temporarily unav= ailable) =20 -=3D=3D Invalid snapshot L1 table =3D=3D +=3D=3D Invalid snapshot L1 table offset =3D=3D Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 wrote 512/512 bytes at offset 0 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) -qemu-img: Failed to load snapshot: Snapshot L1 table too large +qemu-img: Could not open 'TEST_DIR/t.qcow2': Could not read snapshots: Inv= alid argument +Deleting snapshot 1 (test) l1_offset=3D0x400200: L1 table is not cluster a= ligned; snapshot table entry corrupted +Leaked cluster 4 refcount=3D2 reference=3D1 +Leaked cluster 5 refcount=3D2 reference=3D1 +Leaked cluster 6 refcount=3D1 reference=3D0 +Repairing cluster 4 refcount=3D2 reference=3D1 +Repairing cluster 5 refcount=3D2 reference=3D1 +Repairing cluster 6 refcount=3D1 reference=3D0 +Repairing OFLAG_COPIED L2 cluster: l1_index=3D0 l1_entry=3D40000 refcount= =3D1 +Repairing OFLAG_COPIED data cluster: l2_entry=3D50000 refcount=3D1 +The following inconsistencies were found and repaired: + + 3 leaked clusters + 3 corruptions + +Double checking the fixed image now... +No errors were found on the image. + +=3D=3D Invalid snapshot L1 table size =3D=3D +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864 +wrote 512/512 bytes at offset 0 +512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) +qemu-img: Could not open 'TEST_DIR/t.qcow2': Could not read snapshots: Fil= e too large +Deleting snapshot 1 (test) l1_size=3D0x10000000: L1 table is too large; sn= apshot table entry corrupted +Leaked cluster 4 refcount=3D2 reference=3D1 +Leaked cluster 5 refcount=3D2 reference=3D1 +Leaked cluster 6 refcount=3D1 reference=3D0 +Repairing cluster 4 refcount=3D2 reference=3D1 +Repairing cluster 5 refcount=3D2 reference=3D1 +Repairing cluster 6 refcount=3D1 reference=3D0 +Repairing OFLAG_COPIED L2 cluster: l1_index=3D0 l1_entry=3D40000 refcount= =3D1 +Repairing OFLAG_COPIED data cluster: l2_entry=3D50000 refcount=3D1 +The following inconsistencies were found and repaired: + + 3 leaked clusters + 3 corruptions + +Double checking the fixed image now... +No errors were found on the image. *** done --=20 2.11.0