[Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)

[Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)

[Stefan Weil]

The new block was filled with zero when it was allocated by g_malloc0,
but when it was reused later and only partially used, data from the
previously allocated block were still present and written to the new
block.

This caused the problems reported by bug #919242
(https://bugs.launchpad.net/qemu/+bug/919242).

Now the unused parts of the new block which are before and after the data
are always filled with zero, so it is no longer necessary to zero the whole
block with g_malloc0.

I also updated the copyright comment.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 block/vdi.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 31cdfab..6a0011f 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -1,7 +1,7 @@
 /*
  * Block driver for the Virtual Disk Image (VDI) format
  *
- * Copyright (c) 2009 Stefan Weil
+ * Copyright (c) 2009, 2012 Stefan Weil
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -756,15 +756,19 @@ static void vdi_aio_write_cb(void *opaque, int ret)
                  (uint64_t)bmap_entry * s->block_sectors;
         block = acb->block_buffer;
         if (block == NULL) {
-            block = g_malloc0(s->block_size);
+            block = g_malloc(s->block_size);
             acb->block_buffer = block;
             acb->bmap_first = block_index;
             assert(!acb->header_modified);
             acb->header_modified = 1;
         }
         acb->bmap_last = block_index;
+        /* Copy data to be written to new block and zero unused parts. */
+        memset(block, 0, sector_in_block * SECTOR_SIZE);
         memcpy(block + sector_in_block * SECTOR_SIZE,
                acb->buf, n_sectors * SECTOR_SIZE);
+        memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
+               (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
         acb->hd_iov.iov_base = (void *)block;
         acb->hd_iov.iov_len = s->block_size;
         qemu_iovec_init_external(&acb->hd_qiov, &acb->hd_iov, 1);
-- 
1.7.7.3

Re: [Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)

[Stefan Weil]

Am 21.01.2012 13:54, schrieb Stefan Weil:
> The new block was filled with zero when it was allocated by g_malloc0,
> but when it was reused later and only partially used, data from the
> previously allocated block were still present and written to the new
> block.
>
> This caused the problems reported by bug #919242
> (https://bugs.launchpad.net/qemu/+bug/919242).
>
> Now the unused parts of the new block which are before and after the data
> are always filled with zero, so it is no longer necessary to zero the whole
> block with g_malloc0.
>
> I also updated the copyright comment.
>
> Signed-off-by: Stefan Weil<sw@weilnetz.de>
> ---
>   block/vdi.c |    8 ++++++--
>   1 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/block/vdi.c b/block/vdi.c
> index 31cdfab..6a0011f 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -1,7 +1,7 @@
>   /*
>    * Block driver for the Virtual Disk Image (VDI) format
>    *
> - * Copyright (c) 2009 Stefan Weil
> + * Copyright (c) 2009, 2012 Stefan Weil
>    *
>    * This program is free software: you can redistribute it and/or modify
>    * it under the terms of the GNU General Public License as published by
> @@ -756,15 +756,19 @@ static void vdi_aio_write_cb(void *opaque, int ret)
>                    (uint64_t)bmap_entry * s->block_sectors;
>           block = acb->block_buffer;
>           if (block == NULL) {
> -            block = g_malloc0(s->block_size);
> +            block = g_malloc(s->block_size);
>               acb->block_buffer = block;
>               acb->bmap_first = block_index;
>               assert(!acb->header_modified);
>               acb->header_modified = 1;
>           }
>           acb->bmap_last = block_index;
> +        /* Copy data to be written to new block and zero unused parts. */
> +        memset(block, 0, sector_in_block * SECTOR_SIZE);
>           memcpy(block + sector_in_block * SECTOR_SIZE,
>                  acb->buf, n_sectors * SECTOR_SIZE);
> +        memset(block + (sector_in_block + n_sectors) * SECTOR_SIZE, 0,
> +               (s->block_sectors - n_sectors - sector_in_block) * SECTOR_SIZE);
>           acb->hd_iov.iov_base = (void *)block;
>           acb->hd_iov.iov_len = s->block_size;
>           qemu_iovec_init_external(&acb->hd_qiov,&acb->hd_iov, 1);
>    

Hi,

this patch should also be applied to the stable branches of QEMU,
at least to stable-1.0 (after the review, of course).

Regards,
Stefan Weil

Re: [Qemu-devel] [PATCH] block/vdi: Zero unused parts when allocating a new block (fix #919242)

[Kevin Wolf]

Am 21.01.2012 13:54, schrieb Stefan Weil:
> The new block was filled with zero when it was allocated by g_malloc0,
> but when it was reused later and only partially used, data from the
> previously allocated block were still present and written to the new
> block.
> 
> This caused the problems reported by bug #919242
> (https://bugs.launchpad.net/qemu/+bug/919242).
> 
> Now the unused parts of the new block which are before and after the data
> are always filled with zero, so it is no longer necessary to zero the whole
> block with g_malloc0.
> 
> I also updated the copyright comment.
> 
> Signed-off-by: Stefan Weil <sw@weilnetz.de>

Thanks, applied to the block branch (this is not trivial!)

Kevin