Re: [Qemu-devel] [PATCH 1/2] qcow2: add overlap check for bitmap directory

[Eric Blake <eblake@redhat.com>]

[adding Dan in cc]

On 11/30/2017 10:47 AM, Vladimir Sementsov-Ogievskiy wrote:
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>   block/qcow2.h          |  7 +++++--
>   block/qcow2-refcount.c | 12 ++++++++++++
>   block/qcow2.c          |  6 ++++++
>   3 files changed, 23 insertions(+), 2 deletions(-)
> 

Does the LUKS encryption header need similar overlap protection checks?

> diff --git a/block/qcow2.h b/block/qcow2.h
> index 6f0ff15dd0..8f226a3609 100644
> --- a/block/qcow2.h
> +++ b/block/qcow2.h

> +++ b/block/qcow2-refcount.c
> @@ -2585,6 +2585,18 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
>           }
>       }
>   
> +    if ((chk & QCOW2_OL_BITMAP_DIRECTORY) &&
> +        (s->autoclear_features & QCOW2_AUTOCLEAR_BITMAPS))
> +    {
> +        /* update_ext_header_and_dir_in_place firstly drop autoclear flag,
> +         * so it will not fail */

Wording is awkward, how about:

The autoclear flag was previously dropped by 
update_ext_header_and_dir_in_place, so this will not fail

but I'm not sure if that is the intended meaning.

> +        if (overlaps_with(s->bitmap_directory_offset,
> +                          s->bitmap_directory_size))
> +        {
> +            return QCOW2_OL_BITMAP_DIRECTORY;
> +        }
> +    }
> +
>       return 0;
>   }

Do we need to add/update any iotests to cover this?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org