Re: [Qemu-devel] [PATCH v6 09/20] block: vhdx - add region overlap detection for image files

[Stefan Hajnoczi]

On Wed, Sep 25, 2013 at 05:02:54PM -0400, Jeff Cody wrote:
> +/* Check for region overlaps inside the VHDX image */
> +static int vhdx_region_check(BDRVVHDXState *s, uint64_t start, uint64_t 
> length)
> +{
> +    int ret = 0;
> +    uint64_t end;
> +    VHDXRegionEntry *r;
> +
> +    end = start + length;
> +    QLIST_FOREACH(r, &s->regions, entries) {
> +        if ((start >= r->start && start <  r->end) ||
> +            (end   >  r->start && end   <= r->end)) {
> +            ret = -EINVAL;
> +            goto exit;
> +        }
> +    }
> +
> +exit:
> +    return ret;
> +}

This check does not catch a region that spans existing regions:

|----------------| new
     |-----|       existing

This will catch all cases:

QLIST_FOREACH(r, &s->regions, entries) {
    if (!((start >= r->end) || (end <= r->start))) {
        return -EINVAL;
    }
}
return 0;

> @@ -451,6 +499,15 @@ static int vhdx_open_region_tables(BlockDriverState *bs, 
> BDRVVHDXState *s)
>          le32_to_cpus(&rt_entry.length);
>          le32_to_cpus(&rt_entry.data_bits);
>  
> +        /* check for region overlap between these entries, and any
> +         * other memory regions in the file */
> +        ret = vhdx_region_check(s, rt_entry.file_offset, rt_entry.length);
> +        if (ret < 0) {
> +            goto fail;
> +        }
> +
> +        vhdx_region_register(s, rt_entry.file_offset, rt_entry.length);
> +
>          /* see if we recognize the entry */
>          if (guid_eq(rt_entry.guid, bat_guid)) {
>              /* must be unique; if we have already found it this is invalid */
> @@ -481,6 +538,12 @@ static int vhdx_open_region_tables(BlockDriverState *bs, 
> BDRVVHDXState *s)
>              goto fail;
>          }
>      }
> +
> +    if (!bat_rt_found || !metadata_rt_found) {
> +        ret = -EINVAL;
> +        goto fail;
> +    }
> +
>      ret = 0;
>  
>  fail:

Another reason to avoid opening region tables before reading the
journal: we'll add regions twice if the journal had to be flushed.