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.