From 93f0a30fa3a0d68ff54354381859c9904b5573e3 Mon Sep 17 00:00:00 2001 From: Kevin Wolf Date: Tue, 25 Mar 2014 11:45:54 +0100 Subject: [PATCH 36/48] dmg: Fix bdrv_open() error handling RH-Author: Kevin Wolf Message-id: <1395744364-16049-36-git-send-email-kwolf@redhat.com> Patchwork-id: n/a O-Subject: [EMBARGOED RHEL-6.6/6.5.z qemu-kvm PATCH v2 35/45] dmg: Fix bdrv_open() error handling Bugzilla: 1079518 RH-Acked-by: Max Reitz RH-Acked-by: Stefan Hajnoczi RH-Acked-by: Laszlo Ersek Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1079518 Return -errno instead of -1 on errors and add error checks in some places that didn't have one. Passing things by reference requires more correct typing, replaced a few off_ts therefore - with a 32-bit off_t this is even a fix for truncation bugs. While touching the code, fix even some more memory leaks than in the other drivers... Signed-off-by: Kevin Wolf Signed-off-by: Stefan Hajnoczi (cherry picked from commit 69d34a360dfe773e17e72c76d15931c9b9d190f6) Signed-off-by: Kevin Wolf --- block/dmg.c | 129 +++++++++++++++++++++++++++++++++++++++++++---------------- 1 files changed, 94 insertions(+), 35 deletions(-) diff --git a/block/dmg.c b/block/dmg.c index 37902a4..8eb1d69 100644 --- a/block/dmg.c +++ b/block/dmg.c @@ -57,29 +57,42 @@ static int dmg_probe(const uint8_t *buf, int buf_size, const char *filename) return 0; } -static off_t read_off(BlockDriverState *bs, int64_t offset) +static int read_uint64(BlockDriverState *bs, int64_t offset, uint64_t *result) { - uint64_t buffer; - if (bdrv_pread(bs->file, offset, &buffer, 8) < 8) - return 0; - return be64_to_cpu(buffer); + uint64_t buffer; + int ret; + + ret = bdrv_pread(bs->file, offset, &buffer, 8); + if (ret < 0) { + return ret; + } + + *result = be64_to_cpu(buffer); + return 0; } -static off_t read_uint32(BlockDriverState *bs, int64_t offset) +static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result) { - uint32_t buffer; - if (bdrv_pread(bs->file, offset, &buffer, 4) < 4) - return 0; - return be32_to_cpu(buffer); + uint32_t buffer; + int ret; + + ret = bdrv_pread(bs->file, offset, &buffer, 4); + if (ret < 0) { + return ret; + } + + *result = be32_to_cpu(buffer); + return 0; } static int dmg_open(BlockDriverState *bs, int flags) { BDRVDMGState *s = bs->opaque; - off_t info_begin,info_end,last_in_offset,last_out_offset; - uint32_t count; + uint64_t info_begin,info_end,last_in_offset,last_out_offset; + uint32_t count, tmp; uint32_t max_compressed_size=1,max_sectors_per_chunk=1,i; int64_t offset; + int ret; bs->read_only = 1; s->n_chunks = 0; @@ -88,21 +101,32 @@ static int dmg_open(BlockDriverState *bs, int flags) /* read offset of info blocks */ offset = bdrv_getlength(bs->file); if (offset < 0) { + ret = offset; goto fail; } offset -= 0x1d8; - info_begin = read_off(bs, offset); - if (info_begin == 0) { - goto fail; + ret = read_uint64(bs, offset, &info_begin); + if (ret < 0) { + goto fail; + } else if (info_begin == 0) { + ret = -EINVAL; + goto fail; } - if (read_uint32(bs, info_begin) != 0x100) { + ret = read_uint32(bs, info_begin, &tmp); + if (ret < 0) { + goto fail; + } else if (tmp != 0x100) { + ret = -EINVAL; goto fail; } - count = read_uint32(bs, info_begin + 4); - if (count == 0) { + ret = read_uint32(bs, info_begin + 4, &count); + if (ret < 0) { + goto fail; + } else if (count == 0) { + ret = -EINVAL; goto fail; } info_end = info_begin + count; @@ -114,12 +138,20 @@ static int dmg_open(BlockDriverState *bs, int flags) while (offset < info_end) { uint32_t type; - count = read_uint32(bs, offset); - if(count==0) - goto fail; + ret = read_uint32(bs, offset, &count); + if (ret < 0) { + goto fail; + } else if (count == 0) { + ret = -EINVAL; + goto fail; + } offset += 4; - type = read_uint32(bs, offset); + ret = read_uint32(bs, offset, &type); + if (ret < 0) { + goto fail; + } + if (type == 0x6d697368 && count >= 244) { int new_size, chunk_count; @@ -134,8 +166,11 @@ static int dmg_open(BlockDriverState *bs, int flags) s->sectors = g_realloc(s->sectors, new_size); s->sectorcounts = g_realloc(s->sectorcounts, new_size); - for(i=s->n_chunks;in_chunks+chunk_count;i++) { - s->types[i] = read_uint32(bs, offset); + for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) { + ret = read_uint32(bs, offset, &s->types[i]); + if (ret < 0) { + goto fail; + } offset += 4; if(s->types[i]!=0x80000005 && s->types[i]!=1 && s->types[i]!=2) { if(s->types[i]==0xffffffff) { @@ -149,17 +184,31 @@ static int dmg_open(BlockDriverState *bs, int flags) } offset += 4; - s->sectors[i] = last_out_offset+read_off(bs, offset); - offset += 8; - - s->sectorcounts[i] = read_off(bs, offset); - offset += 8; - - s->offsets[i] = last_in_offset+read_off(bs, offset); - offset += 8; - - s->lengths[i] = read_off(bs, offset); - offset += 8; + ret = read_uint64(bs, offset, &s->sectors[i]); + if (ret < 0) { + goto fail; + } + s->sectors[i] += last_out_offset; + offset += 8; + + ret = read_uint64(bs, offset, &s->sectorcounts[i]); + if (ret < 0) { + goto fail; + } + offset += 8; + + ret = read_uint64(bs, offset, &s->offsets[i]); + if (ret < 0) { + goto fail; + } + s->offsets[i] += last_in_offset; + offset += 8; + + ret = read_uint64(bs, offset, &s->lengths[i]); + if (ret < 0) { + goto fail; + } + offset += 8; if(s->lengths[i]>max_compressed_size) max_compressed_size = s->lengths[i]; @@ -173,15 +222,25 @@ static int dmg_open(BlockDriverState *bs, int flags) /* initialize zlib engine */ s->compressed_chunk = g_malloc(max_compressed_size+1); s->uncompressed_chunk = g_malloc(512*max_sectors_per_chunk); - if(inflateInit(&s->zstream) != Z_OK) - goto fail; + if(inflateInit(&s->zstream) != Z_OK) { + ret = -EINVAL; + goto fail; + } s->current_chunk = s->n_chunks; qemu_co_mutex_init(&s->lock); return 0; + fail: - return -1; + g_free(s->types); + g_free(s->offsets); + g_free(s->lengths); + g_free(s->sectors); + g_free(s->sectorcounts); + g_free(s->compressed_chunk); + g_free(s->uncompressed_chunk); + return ret; } static inline int is_sector_in_chunk(BDRVDMGState* s, -- 1.7.1