All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Wolf <kwolf@redhat.com>
To: Alberto Garcia <berto@igalia.com>
Cc: qemu-devel@nongnu.org, qemu-block@nongnu.org,
	Eric Blake <eblake@redhat.com>, Max Reitz <mreitz@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 0/7] Add checks for corruption in the snapshot table
Date: Tue, 6 Mar 2018 15:06:46 +0100	[thread overview]
Message-ID: <20180306140646.GF7139@localhost.localdomain> (raw)
In-Reply-To: <cover.1519921268.git.berto@igalia.com>

Am 01.03.2018 um 17:27 hat Alberto Garcia geschrieben:
> Hey ho!
> 
> As we have already discussed in the mailing list, the offset and size
> values of snapshots' L1 tables are almost never validated in the QEMU
> code.
> 
> One way to deal with this is to validate them when the image is being
> opened (in qcow2_read_snapshots()) and return an error if there's
> anything wrong. However this would render the image unusable because
> we wouldn't be able to try to recover data using the active table as
> we can do now.
> 
> Another possibility is to refuse opening the image but add a way to
> fix it using 'qemu-img check'. But this would be a destructive
> operation, and knowing that the image is already corrupted we can't
> guarantee that we wouldn't be corrupting it even more.
> 
> So although having a way to repair this kind of corruption would be
> nice, we should still allow the user to try to open the image and read
> data from it without requiring a potentially destructive operation
> first.

Generally speaking, there is also always the option to refuse opening a
corrupted image read-write, but to allow opening it read-only. This
would probably still require some checks on use, though (or maybe just
hiding all snapshots).

Kevin

> So this series simply validates the snapshots' L1 tables in the places
> where they are actually used. We already have a function to do this
> kind of checks so we only need to call it.
> 
> Regards,
> 
> Berto
> 
> Alberto Garcia (7):
>   qcow2: Generalize validate_table_offset() into qcow2_validate_table()
>   qcow2: Check L1 table offset in qcow2_snapshot_load_tmp()
>   qcow2: Check L1 table parameters in qcow2_expand_zero_clusters()
>   qcow2: Check snapshot L1 tables in qcow2_check_metadata_overlap()
>   qcow2: Check snapshot L1 table in qcow2_snapshot_goto()
>   qcow2: Check snapshot L1 table in qcow2_snapshot_delete()
>   qcow2: Make qemu-img check detect corrupted L1 tables in snapshots
> 
>  block/qcow2-cluster.c      | 20 +++++++-----
>  block/qcow2-refcount.c     | 24 ++++++++++++++-
>  block/qcow2-snapshot.c     | 21 +++++++++++--
>  block/qcow2.c              | 77 ++++++++++++++++++----------------------------
>  block/qcow2.h              | 10 +++---
>  tests/qemu-iotests/080     | 22 ++++++++++++-
>  tests/qemu-iotests/080.out | 54 ++++++++++++++++++++++++++------
>  7 files changed, 155 insertions(+), 73 deletions(-)
> 
> -- 
> 2.11.0
> 

  parent reply	other threads:[~2018-03-06 14:07 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-01 16:27 [Qemu-devel] [PATCH 0/7] Add checks for corruption in the snapshot table Alberto Garcia
2018-03-01 16:27 ` [Qemu-devel] [PATCH 1/7] qcow2: Generalize validate_table_offset() into qcow2_validate_table() Alberto Garcia
2018-03-01 19:21   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 2/7] qcow2: Check L1 table offset in qcow2_snapshot_load_tmp() Alberto Garcia
2018-03-01 23:32   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 3/7] qcow2: Check L1 table parameters in qcow2_expand_zero_clusters() Alberto Garcia
2018-03-01 23:39   ` Eric Blake
2018-03-06 14:54   ` Kevin Wolf
2018-03-06 15:01     ` Alberto Garcia
2018-03-06 15:11       ` Kevin Wolf
2018-03-06 15:16         ` Alberto Garcia
2018-03-01 16:27 ` [Qemu-devel] [PATCH 4/7] qcow2: Check snapshot L1 tables in qcow2_check_metadata_overlap() Alberto Garcia
2018-03-02  1:20   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 5/7] qcow2: Check snapshot L1 table in qcow2_snapshot_goto() Alberto Garcia
2018-03-02  1:35   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 6/7] qcow2: Check snapshot L1 table in qcow2_snapshot_delete() Alberto Garcia
2018-03-02  1:36   ` Eric Blake
2018-03-01 16:27 ` [Qemu-devel] [PATCH 7/7] qcow2: Make qemu-img check detect corrupted L1 tables in snapshots Alberto Garcia
2018-03-02  1:37   ` Eric Blake
2018-03-06 14:06 ` Kevin Wolf [this message]
2018-03-06 14:18   ` [Qemu-devel] [PATCH 0/7] Add checks for corruption in the snapshot table Alberto Garcia

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180306140646.GF7139@localhost.localdomain \
    --to=kwolf@redhat.com \
    --cc=berto@igalia.com \
    --cc=eblake@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.