Re: [Qemu-devel] [PATCH v2 04/16] qcow2: Keep unknown extra snapshot data

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2201 bytes --]

On 8/19/19 1:55 PM, Max Reitz wrote:
> The qcow2 specification says to ignore unknown extra data fields in
> snapshot table entries.  Currently, we discard it whenever we update the
> image, which is a bit different from "ignore".
> 
> This patch makes the qcow2 driver keep all unknown extra data fields
> when updating an image's snapshot table.
> 

> @@ -80,31 +80,53 @@ int qcow2_read_snapshots(BlockDriverState *bs, Error **errp)
>          sn->date_sec = be32_to_cpu(h.date_sec);
>          sn->date_nsec = be32_to_cpu(h.date_nsec);
>          sn->vm_clock_nsec = be64_to_cpu(h.vm_clock_nsec);
> -        extra_data_size = be32_to_cpu(h.extra_data_size);
> +        sn->extra_data_size = be32_to_cpu(h.extra_data_size);
>  
>          id_str_size = be16_to_cpu(h.id_str_size);
>          name_size = be16_to_cpu(h.name_size);
>  
> -        /* Read extra data */
> +        if (sn->extra_data_size > QCOW_MAX_SNAPSHOT_EXTRA_DATA) {
> +            ret = -EFBIG;
> +            error_setg(errp, "Too much extra metadata in snapshot table "
> +                       "entry %i", i);
> +            goto fail;

We fail if extra_data_size is > 1024...


> +        if (sn->extra_data_size > sizeof(extra)) {
> +            /* Store unknown extra data */
> +            size_t unknown_extra_data_size =
> +                sn->extra_data_size - sizeof(extra);
> +

But read at most 1008 bytes into sn->unknown_extra_data.

> @@ -234,6 +257,22 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
>          }
>          offset += sizeof(extra);
>  
> +        if (sn->extra_data_size > sizeof(extra)) {
> +            size_t unknown_extra_data_size =
> +                sn->extra_data_size - sizeof(extra);
> +
> +            /* qcow2_read_snapshots() ensures no unbounded allocation */
> +            assert(unknown_extra_data_size <= BDRV_REQUEST_MAX_BYTES);

So this assertion is quite loose in what it permits; tighter would be

assert(unknown_extra_data_size <= QCOW_MAX_SNAPSHOT_EXTRA_DATA -
sizeof(extra))

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]