On Tue, Apr 23, 2013 at 10:24:22AM -0400, Jeff Cody wrote: > + if (!vhdx_checksum_is_valid(buffer, VHDX_HEADER_BLOCK_SIZE, 4) || > + s->rt.signature != VHDX_RT_MAGIC) { > + ret = -EINVAL; > + goto fail; > + } > + > + for (i = 0; i < s->rt.entry_count; i++) {
It's nice to avoid signed/unsigned comparisons. i should be uint32_t just like entry_count. > + memcpy(&rt_entry, buffer+offset, sizeof(rt_entry)); > + offset += sizeof(rt_entry); Looks like we're trusting rt.entry_count to be a sane value? Need to prevent offset from exceeding buffer size. > + while (logical_sector_size >>= 1) { > + s->logical_sector_size_bits++; > + } > + while (sectors_per_block >>= 1) { > + s->sectors_per_block_bits++; > + } > + while (chunk_ratio >>= 1) { > + s->chunk_ratio_bits++; > + } > + while (block_size >>= 1) { > + s->block_size_bits++; > + } ctz()/clo() do this. > +static int vhdx_parse_log(BlockDriverState *bs, BDRVVHDXState *s) > +{ > + int ret = 0; > + int i; > + vhdx_header *hdr; > + > + hdr = s->headers[s->curr_header]; > + > + /* either either the log guid, or log length is zero, either either > + s->bat_offset = s->bat_rt.file_offset; > + s->bat_entries = s->bat_rt.length / sizeof(vhdx_bat_entry); > + s->bat = qemu_blockalign(bs, s->bat_rt.length); No sanity check was done on bat_rt.length. If this allocation fails QEMU will exit. Could be used as a DoS if you can get someone to attach a malicious VHDX to their VM?