| Message ID | 1371035190-13328-1-git-send-email-evgeny.budilovsky@ravellosystems.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Jun 12, 2013 at 02:06:30PM +0300, Evgeny Budilovsky wrote: > @@ -719,27 +719,40 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, > int64_t desc_offset) > { > int ret; > - char buf[2048]; > + char *buf = NULL; > char ct[128]; > BDRVVmdkState *s = bs->opaque; > + int64_t size; > > - ret = bdrv_pread(bs->file, desc_offset, buf, sizeof(buf)); > + size = bdrv_getlength(bs->file); > + if (size < 0) { > + return -EINVAL; > + } > + > + size = MIN(size, 1 << 20); /* avoid unbounded allocation */ I think this is okay, initially I was worried that this function might be called to probe files which we have not identified as descriptor files yet (they could be big!). But looking at the callers, it should be reasonable to read up to 1 MB. (Would have been bad to read 1 MB just to check if this parses as a descriptor file.) > + buf = g_malloc0(size + 1); > + > + ret = bdrv_pread(bs->file, desc_offset, buf, size); > if (ret < 0) { > - return ret; > + goto exit; > } > - buf[2047] = '\0'; Thanks for pointing out the g_malloc0(), I missed it. Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Am 12.06.2013 um 13:06 hat Evgeny Budilovsky geschrieben: > the hard-coded 2k buffer on the stack won't allow reading big descriptor > files which can be generated when storing big images. For example 500G > vmdk splitted to 2G chunks. > > Signed-off-by: Evgeny Budilovsky <evgeny.budilovsky@ravellosystems.com> Thanks, applied to the block branch. Kevin
diff --git a/block/vmdk.c b/block/vmdk.c index 608daaf..751b63b 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -719,27 +719,40 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags, int64_t desc_offset) { int ret; - char buf[2048]; + char *buf = NULL; char ct[128]; BDRVVmdkState *s = bs->opaque; + int64_t size; - ret = bdrv_pread(bs->file, desc_offset, buf, sizeof(buf)); + size = bdrv_getlength(bs->file); + if (size < 0) { + return -EINVAL; + } + + size = MIN(size, 1 << 20); /* avoid unbounded allocation */ + buf = g_malloc0(size + 1); + + ret = bdrv_pread(bs->file, desc_offset, buf, size); if (ret < 0) { - return ret; + goto exit; } - buf[2047] = '\0'; if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) { - return -EMEDIUMTYPE; + ret = -EMEDIUMTYPE; + goto exit; } if (strcmp(ct, "monolithicFlat") && strcmp(ct, "twoGbMaxExtentSparse") && strcmp(ct, "twoGbMaxExtentFlat")) { fprintf(stderr, "VMDK: Not supported image type \"%s\""".\n", ct); - return -ENOTSUP; + ret = -ENOTSUP; + goto exit; } s->desc_offset = 0; - return vmdk_parse_extents(buf, bs, bs->file->filename); + ret = vmdk_parse_extents(buf, bs, bs->file->filename); +exit: + g_free(buf); + return ret; } static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
the hard-coded 2k buffer on the stack won't allow reading big descriptor files which can be generated when storing big images. For example 500G vmdk splitted to 2G chunks. Signed-off-by: Evgeny Budilovsky <evgeny.budilovsky@ravellosystems.com> --- block/vmdk.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) -- 1.7.9.5