[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling
From: |
Blue Swirl |
Subject: |
Re: [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling |
Date: |
Sat, 2 Feb 2013 12:47:18 +0000 |
On Fri, Feb 1, 2013 at 2:28 PM, Stefan Hajnoczi <address@hidden> wrote:
> From: Kevin Wolf <address@hidden>
>
> 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 <address@hidden>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
> block/dmg.c | 135
> +++++++++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 97 insertions(+), 38 deletions(-)
>
> diff --git a/block/dmg.c b/block/dmg.c
> index ac397dc..53be25d 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;i<s->n_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) {
Please add a space between 'if' and '('.
> + 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.8.1
>
>
- [Qemu-devel] [PULL for-1.4 00/13] Block patches, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 01/13] qemu-iotests: Add regression test for b7ab0fea, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 02/13] block: Fix is_allocated_above with resized files, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 03/13] block: Adds mirroring tests for resized images, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 04/13] vmdk: Allow selecting SCSI adapter in image creation, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 05/13] sheepdog: pass vdi_id to sheep daemon for sd_close(), Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 07/13] cloop: Fix bdrv_open() error handling, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 06/13] bochs: Fix bdrv_open() error handling, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling, Stefan Hajnoczi, 2013/02/01
- Re: [Qemu-devel] [PATCH 09/13] dmg: Fix bdrv_open() error handling,
Blue Swirl <=
- [Qemu-devel] [PATCH 08/13] vpc: Fix bdrv_open() error handling, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 10/13] dmg: Use g_free instead of free, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 11/13] parallels: Fix bdrv_open() error handling, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 12/13] vmdk: Allow space in file name, Stefan Hajnoczi, 2013/02/01
- [Qemu-devel] [PATCH 13/13] block/raw-posix: Build fix for O_ASYNC, Stefan Hajnoczi, 2013/02/01