Am 26.08.2013 um 15:04 hat Max Reitz geschrieben:
> Two new functions are added; the first one checks a given range in the
> image file for overlaps with metadata (main header, L1 tables, L2
> tables, refcount table and blocks).
>
> The second one should be used immediately before writing to the image
> file as it calls the first function and, upon collision, marks the
> image as corrupt and makes the BDS unusable, thereby preventing
> further access.
>
> Both functions take a bitmask argument specifying the structures which
> should be checked for overlaps, making it possible to also check
> metadata writes against colliding with other structures.
>
> Signed-off-by: Max Reitz <mre...@redhat.com>
> ---
> block/qcow2-refcount.c | 142
> +++++++++++++++++++++++++++++++++++++++++++++++++
> block/qcow2.h | 28 ++++++++++
> 2 files changed, 170 insertions(+)
>
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 1244693..c8141c8 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -25,6 +25,7 @@
> #include "qemu-common.h"
> #include "block/block_int.h"
> #include "block/qcow2.h"
> +#include "qemu/range.h"
>
> static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
> static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
> @@ -1372,3 +1373,144 @@ fail:
> return ret;
> }
>
> +/*
> + * Checks if the given offset into the image file is actually free to use by
> + * looking for overlaps with important metadata sections (L1/L2 tables etc.),
> + * i.e. a sanity check without relying on the refcount tables.
> + *
> + * The chk parameter specifies exactly what checks to perform.
> + *
> + * Returns:
> + * - 0 if writing to this offset will not affect the mentioned metadata
> + * - a positive QCow2MetadataOverlap value indicating one overlapping section
> + * - a negative value (-errno) indicating an error while performing a check,
> + * e.g. when bdrv_read failed on QCOW2_OL_INACTIVE_L2
> + */
> +int qcow2_check_metadata_overlap(BlockDriverState *bs, QCow2MetadataOverlap
> chk,
chk is really just an int, because you don't pass a single enum value but
a bit mask consisting of multiple enum values ored together.
> + int64_t offset, int64_t size)
> +{
> + BDRVQcowState *s = bs->opaque;
> + int i, j;
> +
> + if (!size) {
> + return 0;
> + }
> +
> + if (chk & QCOW2_OL_MAIN_HEADER) {
> + if (offset < s->cluster_size) {
> + return QCOW2_OL_MAIN_HEADER;
> + }
> + }
> +
> + if ((chk & QCOW2_OL_ACTIVE_L1) && s->l1_size) {
> + if (ranges_overlap(offset, size, s->l1_table_offset,
> + s->l1_size * sizeof(uint64_t))) {
The size could be rounded up to the next cluster boundary (same thing
for other metadata types).
> + return QCOW2_OL_ACTIVE_L1;
> + }
> + }
> +
> + if ((chk & QCOW2_OL_REFCOUNT_TABLE) && s->refcount_table_size) {
> + if (ranges_overlap(offset, size, s->refcount_table_offset,
> + s->refcount_table_size * sizeof(uint64_t))) {
> + return QCOW2_OL_REFCOUNT_TABLE;
> + }
> + }
> +
> + if ((chk & QCOW2_OL_SNAPSHOT_TABLE) && s->snapshots_size) {
> + if (ranges_overlap(offset, size, s->snapshots_offset,
> + s->snapshots_size)) {
> + return QCOW2_OL_SNAPSHOT_TABLE;
> + }
> + }
> +
> + if ((chk & QCOW2_OL_INACTIVE_L1) && s->snapshots) {
> + for (i = 0; i < s->nb_snapshots; i++) {
> + if (s->snapshots[i].l1_size &&
> + ranges_overlap(offset, size, s->snapshots[i].l1_table_offset,
> + s->snapshots[i].l1_size * sizeof(uint64_t))) {
> + return QCOW2_OL_INACTIVE_L1;
> + }
> + }
> + }
> +
> + if ((chk & QCOW2_OL_ACTIVE_L2) && s->l1_table) {
> + for (i = 0; i < s->l1_size; i++) {
> + if ((s->l1_table[i] & L1E_OFFSET_MASK) &&
> + ranges_overlap(offset, size, s->l1_table[i] &
> L1E_OFFSET_MASK,
> + s->cluster_size)) {
> + return QCOW2_OL_ACTIVE_L2;
> + }
> + }
> + }
> +
> + if ((chk & QCOW2_OL_REFCOUNT_BLOCK) && s->refcount_table) {
> + for (i = 0; i < s->refcount_table_size; i++) {
> + if ((s->refcount_table[i] & REFT_OFFSET_MASK) &&
> + ranges_overlap(offset, size, s->refcount_table[i] &
> + REFT_OFFSET_MASK, s->cluster_size)) {
> + return QCOW2_OL_REFCOUNT_BLOCK;
> + }
> + }
> + }
> +
> + if ((chk & QCOW2_OL_INACTIVE_L2) && s->snapshots) {
> + for (i = 0; i < s->nb_snapshots; i++) {
> + uint64_t l1_ofs = s->snapshots[i].l1_table_offset;
> + uint32_t l1_sz = s->snapshots[i].l1_size;
> + uint64_t *l1 = g_malloc(l1_sz * sizeof(uint64_t));
> + int ret;
> +
> + ret = bdrv_read(bs->file, l1_ofs / BDRV_SECTOR_SIZE, (uint8_t
> *)l1,
> + l1_sz * sizeof(uint64_t) / BDRV_SECTOR_SIZE);
> +
> + if (ret < 0) {
> + g_free(l1);
> + return ret;
> + }
> +
> + for (j = 0; j < l1_sz; j++) {
> + if ((l1[j] & L1E_OFFSET_MASK) &&
> + ranges_overlap(offset, size, l1[j] & L1E_OFFSET_MASK,
> + s->cluster_size)) {
> + g_free(l1);
> + return QCOW2_OL_INACTIVE_L2;
> + }
> + }
> +
> + g_free(l1);
> + }
> + }
> +
> + return 0;
> +}
> +
> +/*
> + * First performs a check for metadata overlaps (through
> + * qcow2_check_metadata_overlap); if that fails with a negative value (error
> + * while performing a check), it will print a message but otherwise ignore
> that
> + * error. If an impending overlap is detected, the BDS will be made unusable
> and
> + * the qcow2 file marked corrupt.
> + *
> + * Returns 0 if there were no overlaps (or an error occured while checking
> for
> + * overlaps) or a positive QCow2MetadataOverlap value on overlap (then, the
> BDS
> + * will be unusable and the qcow2 file marked corrupt).
> + */
> +int qcow2_pre_write_overlap_check(BlockDriverState *bs, QCow2MetadataOverlap
> chk,
> + int64_t offset, int64_t size)
> +{
> + int ret = qcow2_check_metadata_overlap(bs, chk, offset, size);
> +
> + if (ret < 0) {
> + fprintf(stderr, "qcow2: Error while checking for metadata overlaps: "
> + "%s\n", strerror(-ret));
Leftover debug code?
> + return ret;
> + } else if (ret > 0) {
> + fprintf(stderr, "qcow2: Preventing invalid write on metadata; "
> + "image marked as corrupt.\n");
This one makes actually sense to keep even for production as it is a
condition that we want to make sure to appear in log files.
Another thing to consider would be to send out a QMP event when this
happens.
> + qcow2_mark_corrupt(bs);
> + bs->drv = NULL; /* make BDS unusable */
> + return ret;
> + }
> +
> + return 0;
> +}
Kevin
