[Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

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 |   28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..1bc944b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
+    if (size < 0) {
+        return -EINVAL;
+    }
+
+    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:
+    if (buf) {
+        g_free(buf);
+    }
+    return ret;
 }

 static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
--
1.7.9.5

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Don Slutz]

On 06/12/13 03:08, Evgeny Budilovsky wrote:
> 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 |   28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 608daaf..1bc944b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
> +    if (size < 0) {
> +        return -EINVAL;
> +    }
> +
While this is right for vmdk splitted to 2G chunks, I think this will 
fail for a big enough "monolithicFlat" vmdk where there is only the 1 
file (g_malloc() will most likely fail for a 500GB 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';
>       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:
> +    if (buf) {
> +        g_free(buf);
> +    }
> +    return ret;
>   }
>
>   static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
> --
> 1.7.9.5
>
    -Don Slutz

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

[-- Attachment #1: Type: text/plain, Size: 2832 bytes --]

On Fri, Jun 14, 2013 at 12:15 AM, Don Slutz <dslutz@verizon.com> wrote:

> On 06/12/13 03:08, Evgeny Budilovsky wrote:
>
>> 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<evgeny.budilovsky@ravellosystems.com>
>> >
>> ---
>>   block/vmdk.c |   28 +++++++++++++++++++++-------
>>   1 file changed, 21 insertions(+), 7 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 608daaf..1bc944b 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -719,27 +719,41 @@ 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_get_allocated_file_size(**bs);
>> +    if (size < 0) {
>> +        return -EINVAL;
>> +    }
>> +
>>
> While this is right for vmdk splitted to 2G chunks, I think this will fail
> for a big enough "monolithicFlat" vmdk where there is only the 1 file
> (g_malloc() will most likely fail for a 500GB file).
>
> With the "monolithicFlat" vmdk the descriptor file is a small textual
file. So this code should work. In the second version of this patch I've
added some constraint to the allocation size just in case the file is
corrupted or we have misinterpreted the format.

size = MIN(size, 1 << 20);  /* avoid unbounded allocation */
buf = g_malloc0(size + 1);

 +    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:
>> +    if (buf) {
>> +        g_free(buf);
>> +    }
>> +    return ret;
>>   }
>>
>>   static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
>> --
>> 1.7.9.5
>>
>>     -Don Slutz
>



-- 
Best Regards,
Evgeny

[-- Attachment #2: Type: text/html, Size: 4409 bytes --]

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Don Slutz]

[-- Attachment #1: Type: text/plain, Size: 3606 bytes --]

On 06/14/13 02:41, Evgeny Budilovsky wrote:
>
>
>
> On Fri, Jun 14, 2013 at 12:15 AM, Don Slutz <dslutz@verizon.com 
> <mailto:dslutz@verizon.com>> wrote:
>
>     On 06/12/13 03:08, Evgeny Budilovsky wrote:
>
>         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
>         <mailto:evgeny.budilovsky@ravellosystems.com>>
>         ---
>           block/vmdk.c |   28 +++++++++++++++++++++-------
>           1 file changed, 21 insertions(+), 7 deletions(-)
>
>         diff --git a/block/vmdk.c b/block/vmdk.c
>         index 608daaf..1bc944b 100644
>         --- a/block/vmdk.c
>         +++ b/block/vmdk.c
>         @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
>         +    if (size < 0) {
>         +        return -EINVAL;
>         +    }
>         +
>
>     While this is right for vmdk splitted to 2G chunks, I think this
>     will fail for a big enough "monolithicFlat" vmdk where there is
>     only the 1 file (g_malloc() will most likely fail for a 500GB file).
>
> With the "monolithicFlat" vmdk the descriptor file is a small textual 
> file. So this code should work. In the second version of this patch 
> I've added some constraint to the allocation size just in case the 
> file is corrupted or we have misinterpreted the format.
>
Opps, I did the wrong one.  Both createType="streamOptimized" and 
createType="monolithicSparse" are only 1 file.
> size = MIN(size, 1 << 20);  /* avoid unbounded allocation */
This will "fix" the issue.
    -Don Slutz
> buf = g_malloc0(size + 1);
>
>         +    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:
>         +    if (buf) {
>         +        g_free(buf);
>         +    }
>         +    return ret;
>           }
>
>           static int vmdk_open(BlockDriverState *bs, QDict *options,
>         int flags)
>         --
>         1.7.9.5
>
>        -Don Slutz
>
>
>
>
> -- 
> Best Regards,
> Evgeny


[-- Attachment #2: Type: text/html, Size: 8711 bytes --]

[Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

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 |   28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..1bc944b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
+    if (size < 0) {
+        return -EINVAL;
+    }
+
+    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:
+    if (buf) {
+        g_free(buf);
+    }
+    return ret;
 }

 static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
--
1.7.9.5

[-- Attachment #2: Type: text/html, Size: 2773 bytes --]

[Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

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 |   28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 608daaf..1bc944b 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
+    if (size < 0) {
+        return -EINVAL;
+    }
+
+    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:
+    if (buf) {
+        g_free(buf);
+    }
+    return ret;
 }

 static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
--
1.7.9.5

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Stefan Hajnoczi]

On Wed, Jun 12, 2013 at 11:04:44AM +0300, Evgeny Budilovsky wrote:
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 608daaf..1bc944b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);

Please use bdrv_getlength() instead of bdrv_get_allocated_file_size(),
which checks stat.st_blocks.  From the stat(2) man page:

  The st_blocks field indicates the number of blocks  allocated  to
  the  file,  512-byte  units.  (This may be smaller than st_size/512
  when the file has holes.).

> +    if (size < 0) {
> +        return -EINVAL;
> +    }
> +
> +    buf = g_malloc0(size+1);

Spaces please:
g_malloc0(size + 1)

> +
> +    ret = bdrv_pread(bs->file, desc_offset, buf, size);
>      if (ret < 0) {
> -        return ret;
> +        goto exit;
>      }
> -    buf[2047] = '\0';

The buffer must be NUL-terminated.

>      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:
> +    if (buf) {
> +        g_free(buf);
> +    }

The if is not necessary since g_free(NULL) is a nop.

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

[-- Attachment #1: Type: text/plain, Size: 2312 bytes --]

On Wed, Jun 12, 2013 at 1:17 PM, Stefan Hajnoczi <stefanha@redhat.com>wrote:

> On Wed, Jun 12, 2013 at 11:04:44AM +0300, Evgeny Budilovsky wrote:
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 608daaf..1bc944b 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
>
> Please use bdrv_getlength() instead of bdrv_get_allocated_file_size(),
> which checks stat.st_blocks.  From the stat(2) man page:
>
>   The st_blocks field indicates the number of blocks  allocated  to
>   the  file,  512-byte  units.  (This may be smaller than st_size/512
>   when the file has holes.).
>
> applied

>  > +    if (size < 0) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    buf = g_malloc0(size+1);
>
> Spaces please:
> g_malloc0(size + 1)
>
> applied

>  > +
> > +    ret = bdrv_pread(bs->file, desc_offset, buf, size);
> >      if (ret < 0) {
> > -        return ret;
> > +        goto exit;
> >      }
> > -    buf[2047] = '\0';
>
> The buffer must be NUL-terminated.
>
> g_malloc0 allocates buffer which is zero initialized so I can skeep the
null termination


>  >      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:
> > +    if (buf) {
> > +        g_free(buf);
> > +    }
>
> The if is not necessary since g_free(NULL) is a nop.
>

applied

[-- Attachment #2: Type: text/html, Size: 3941 bytes --]

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Kevin Wolf]

Am 12.06.2013 um 10:04 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>
> ---
>  block/vmdk.c |   28 +++++++++++++++++++++-------
>  1 file changed, 21 insertions(+), 7 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 608daaf..1bc944b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
> +    if (size < 0) {
> +        return -EINVAL;
> +    }
> +
> +    buf = g_malloc0(size+1);

This is an unbounded allocation. Not sure if this is a good idea. Can we
restrict the maximum size to something reasonably small, like a megabyte?

Kevin

Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Evgeny Budilovsky]

[-- Attachment #1: Type: text/plain, Size: 1388 bytes --]

On Wed, Jun 12, 2013 at 1:30 PM, Kevin Wolf <kwolf@redhat.com> wrote:

> Am 12.06.2013 um 10:04 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>
> > ---
> >  block/vmdk.c |   28 +++++++++++++++++++++-------
> >  1 file changed, 21 insertions(+), 7 deletions(-)
> >
> > diff --git a/block/vmdk.c b/block/vmdk.c
> > index 608daaf..1bc944b 100644
> > --- a/block/vmdk.c
> > +++ b/block/vmdk.c
> > @@ -719,27 +719,41 @@ 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_get_allocated_file_size(bs);
> > +    if (size < 0) {
> > +        return -EINVAL;
> > +    }
> > +
> > +    buf = g_malloc0(size+1);
>
> This is an unbounded allocation. Not sure if this is a good idea. Can we
> restrict the maximum size to something reasonably small, like a megabyte?
>
> Kevin
>

yes good idea !

-- 
Best Regards,
Evgeny

[-- Attachment #2: Type: text/html, Size: 2099 bytes --]