[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 56/69] qcow2: Fix overly long snapshot tables
From: |
Max Reitz |
Subject: |
[PULL 56/69] qcow2: Fix overly long snapshot tables |
Date: |
Mon, 28 Oct 2019 13:14:48 +0100 |
We currently refuse to open qcow2 images with overly long snapshot
tables. This patch makes qemu-img check -r all drop all offending
entries past what we deem acceptable.
The user cannot choose which snapshots are removed. This is fine
because we have chosen the maximum snapshot table size to be so large
(64 MB) that it cannot be reasonably reached. If the snapshot table
exceeds this size, the image has probably been corrupted in some way; in
this case, it is most important to just make the image usable such that
the user can copy off at least the active layer.
(Also note that the snapshots will be removed only with "-r all", so a
plain "check" or "check -r leaks" will not delete any data.)
Signed-off-by: Max Reitz <address@hidden>
Reviewed-by: Eric Blake <address@hidden>
Message-id: address@hidden
Signed-off-by: Max Reitz <address@hidden>
---
block/qcow2-snapshot.c | 88 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 78 insertions(+), 10 deletions(-)
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 582eb3386a..366d9f574c 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -29,15 +29,24 @@
#include "qemu/error-report.h"
#include "qemu/cutils.h"
+static void qcow2_free_single_snapshot(BlockDriverState *bs, int i)
+{
+ BDRVQcow2State *s = bs->opaque;
+
+ assert(i >= 0 && i < s->nb_snapshots);
+ g_free(s->snapshots[i].name);
+ g_free(s->snapshots[i].id_str);
+ g_free(s->snapshots[i].unknown_extra_data);
+ memset(&s->snapshots[i], 0, sizeof(s->snapshots[i]));
+}
+
void qcow2_free_snapshots(BlockDriverState *bs)
{
BDRVQcow2State *s = bs->opaque;
int i;
for(i = 0; i < s->nb_snapshots; i++) {
- g_free(s->snapshots[i].name);
- g_free(s->snapshots[i].id_str);
- g_free(s->snapshots[i].unknown_extra_data);
+ qcow2_free_single_snapshot(bs, i);
}
g_free(s->snapshots);
s->snapshots = NULL;
@@ -48,6 +57,14 @@ void qcow2_free_snapshots(BlockDriverState *bs)
* If @repair is true, try to repair a broken snapshot table instead
* of just returning an error:
*
+ * - If the snapshot table was too long, set *nb_clusters_reduced to
+ * the number of snapshots removed off the end.
+ * The caller will update the on-disk nb_snapshots accordingly;
+ * this leaks clusters, but is safe.
+ * (The on-disk information must be updated before
+ * qcow2_check_refcounts(), because that function relies on
+ * s->nb_snapshots to reflect the on-disk value.)
+ *
* - If there were snapshots with too much extra metadata, increment
* *extra_data_dropped for each.
* This requires the caller to eventually rewrite the whole snapshot
@@ -59,6 +76,7 @@ void qcow2_free_snapshots(BlockDriverState *bs)
* extra data.)
*/
static int qcow2_do_read_snapshots(BlockDriverState *bs, bool repair,
+ int *nb_clusters_reduced,
int *extra_data_dropped,
Error **errp)
{
@@ -67,7 +85,7 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, bool
repair,
QCowSnapshotExtraData extra;
QCowSnapshot *sn;
int i, id_str_size, name_size;
- int64_t offset;
+ int64_t offset, pre_sn_offset;
uint64_t table_length = 0;
int ret;
@@ -83,6 +101,7 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs,
bool repair,
for(i = 0; i < s->nb_snapshots; i++) {
bool truncate_unknown_extra_data = false;
+ pre_sn_offset = offset;
table_length = ROUND_UP(table_length, 8);
/* Read statically sized part of the snapshot header */
@@ -197,9 +216,31 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs,
bool repair,
if (table_length > QCOW_MAX_SNAPSHOTS_SIZE ||
offset - s->snapshots_offset > INT_MAX)
{
- ret = -EFBIG;
- error_setg(errp, "Snapshot table is too big");
- goto fail;
+ if (!repair) {
+ ret = -EFBIG;
+ error_setg(errp, "Snapshot table is too big");
+ error_append_hint(errp, "You can force-remove all %u "
+ "overhanging snapshots with qemu-img check "
+ "-r all\n", s->nb_snapshots - i);
+ goto fail;
+ }
+
+ fprintf(stderr, "Discarding %u overhanging snapshots (snapshot "
+ "table is too big)\n", s->nb_snapshots - i);
+
+ *nb_clusters_reduced += (s->nb_snapshots - i);
+
+ /* Discard current snapshot also */
+ qcow2_free_single_snapshot(bs, i);
+
+ /*
+ * This leaks all the rest of the snapshot table and the
+ * snapshots' clusters, but we run in check -r all mode,
+ * so qcow2_check_refcounts() will take care of it.
+ */
+ s->nb_snapshots = i;
+ offset = pre_sn_offset;
+ break;
}
}
@@ -214,7 +255,7 @@ fail:
int qcow2_read_snapshots(BlockDriverState *bs, Error **errp)
{
- return qcow2_do_read_snapshots(bs, false, NULL, errp);
+ return qcow2_do_read_snapshots(bs, false, NULL, NULL, errp);
}
/* add at the end of the file a new list of snapshots */
@@ -382,6 +423,7 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
{
BDRVQcow2State *s = bs->opaque;
Error *local_err = NULL;
+ int nb_clusters_reduced = 0;
int extra_data_dropped = 0;
int ret;
struct {
@@ -419,7 +461,8 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
qemu_co_mutex_unlock(&s->lock);
ret = qcow2_do_read_snapshots(bs, fix & BDRV_FIX_ERRORS,
- &extra_data_dropped, &local_err);
+ &nb_clusters_reduced, &extra_data_dropped,
+ &local_err);
qemu_co_mutex_lock(&s->lock);
if (ret < 0) {
result->check_errors++;
@@ -432,7 +475,32 @@ int coroutine_fn
qcow2_check_read_snapshot_table(BlockDriverState *bs,
return ret;
}
- result->corruptions += extra_data_dropped;
+ result->corruptions += nb_clusters_reduced + extra_data_dropped;
+
+ if (nb_clusters_reduced) {
+ /*
+ * Update image header now, because:
+ * (1) qcow2_check_refcounts() relies on s->nb_snapshots to be
+ * the same as what the image header says,
+ * (2) this leaks clusters, but qcow2_check_refcounts() will
+ * fix that.
+ */
+ assert(fix & BDRV_FIX_ERRORS);
+
+ snapshot_table_pointer.nb_snapshots = cpu_to_be32(s->nb_snapshots);
+ ret = bdrv_pwrite_sync(bs->file, offsetof(QCowHeader, nb_snapshots),
+ &snapshot_table_pointer.nb_snapshots,
+ sizeof(snapshot_table_pointer.nb_snapshots));
+ if (ret < 0) {
+ result->check_errors++;
+ fprintf(stderr, "ERROR failed to update the snapshot count in the "
+ "image header: %s\n", strerror(-ret));
+ return ret;
+ }
+
+ result->corruptions_fixed += nb_clusters_reduced;
+ result->corruptions -= nb_clusters_reduced;
+ }
return 0;
}
--
2.21.0
- [PULL 48/69] qcow2: Keep unknown extra snapshot data, (continued)
- [PULL 48/69] qcow2: Keep unknown extra snapshot data, Max Reitz, 2019/10/28
- [PULL 62/69] block/cor: Drop cor_co_truncate(), Max Reitz, 2019/10/28
- [PULL 59/69] iotests: Add peek_file* functions, Max Reitz, 2019/10/28
- [PULL 52/69] qcow2: Separate qcow2_check_read_snapshot_table(), Max Reitz, 2019/10/28
- [PULL 54/69] qcow2: Fix broken snapshot table entries, Max Reitz, 2019/10/28
- [PULL 57/69] qcow2: Repair snapshot table with too many entries, Max Reitz, 2019/10/28
- [PULL 55/69] qcow2: Keep track of the snapshot table length, Max Reitz, 2019/10/28
- [PULL 58/69] qcow2: Fix v3 snapshot table entry compliancy, Max Reitz, 2019/10/28
- [PULL 61/69] block: Handle filter truncation like native impl., Max Reitz, 2019/10/28
- [PULL 63/69] block: Do not truncate file node when formatting, Max Reitz, 2019/10/28
- [PULL 56/69] qcow2: Fix overly long snapshot tables,
Max Reitz <=
- [PULL 65/69] block: Evaluate @exact in protocol drivers, Max Reitz, 2019/10/28
- [PULL 60/69] iotests: Test qcow2's snapshot table handling, Max Reitz, 2019/10/28
- [PULL 64/69] block: Add @exact parameter to bdrv_co_truncate(), Max Reitz, 2019/10/28
- [PULL 67/69] block: Pass truncate exact=true where reasonable, Max Reitz, 2019/10/28
- [PULL 66/69] block: Let format drivers pass @exact, Max Reitz, 2019/10/28
- [PULL 69/69] qemu-iotests: restrict 264 to qcow2 only, Max Reitz, 2019/10/28
- [PULL 68/69] Revert "qemu-img: Check post-truncation size", Max Reitz, 2019/10/28
- Re: [PULL 00/69] Block patches for softfreeze, Peter Maydell, 2019/10/28