[Qemu-devel] [PATCH v3 00/21] block: Handle failure for potentially large allocations

[Qemu-devel] [PATCH v3 00/21] block: Handle failure for potentially large allocations

[Kevin Wolf]

A not too small part of the recent CVEs were DoS scenarios by letting
qemu abort with too large memory allocations. We generally "fixed" these
cases by setting some limits on values read from image files that
influence the size of allocations.

Because we still need to allow reading large images, this works only to
a certain degree and we still can get fairly large allocations, which
are not unthinkable to fail on some machines.

This series converts potentially large allocations to g_try_malloc() and
friends and handles failure gracefully e.g. by returning -ENOMEM. This
may cause hot-plug of a new disk or individual requests to fail, but the
VM as a whole can keep running.

v3:
- Changed qemu_try_blockalign() to only return NULL on failure. size = 0
  results in a small allocation now (size of the alignment) [Benoît]
- Patch 8 (nfs): Check for size != 0 before failing [Benoît]
- Patch 11 (qcow2):
  * Fix memory leak in alloc_refcount_block() [Max]
  * Report internal error for -ENOMEM in qcow2_check() [Max]
- Patch 15 (rbd): Build fix [Markus]

v2:
- Some more places check for size = 0 before they treat NULL as an error
- Patch 2 (block.c): Added missing NULL return check for
  qemu_try_blockalign() [Stefan]
- Patch 7 (iscsi): Fixed acb->task memory leak [Stefan]
- For conversions from g_malloc() to qemu_try_blockalign(), made sure to
  be consistent about pairing the latter with qemu_vfree() [Stefan]

Kevin Wolf (20):
  block: Introduce qemu_try_blockalign()
  block: Handle failure for potentially large allocations
  bochs: Handle failure for potentially large allocations
  cloop: Handle failure for potentially large allocations
  curl: Handle failure for potentially large allocations
  dmg: Handle failure for potentially large allocations
  iscsi: Handle failure for potentially large allocations
  nfs: Handle failure for potentially large allocations
  parallels: Handle failure for potentially large allocations
  qcow1: Handle failure for potentially large allocations
  qcow2: Handle failure for potentially large allocations
  qed: Handle failure for potentially large allocations
  raw-posix: Handle failure for potentially large allocations
  raw-win32: Handle failure for potentially large allocations
  rbd: Handle failure for potentially large allocations
  vdi: Handle failure for potentially large allocations
  vhdx: Handle failure for potentially large allocations
  vmdk: Handle failure for potentially large allocations
  vpc: Handle failure for potentially large allocations
  mirror: Handle failure for potentially large allocations

Max Reitz (1):
  qcow2: Return useful error code in refcount_init()

 block.c                | 47 ++++++++++++++++++++++++++++++++++++-------
 block/bochs.c          |  6 +++++-
 block/cloop.c          | 23 ++++++++++++++++++---
 block/curl.c           |  8 +++++++-
 block/dmg.c            | 19 ++++++++++++------
 block/iscsi.c          | 17 +++++++++++++---
 block/mirror.c         |  7 ++++++-
 block/nfs.c            |  6 +++++-
 block/parallels.c      |  6 +++++-
 block/qcow.c           | 33 +++++++++++++++++++++++-------
 block/qcow2-cache.c    | 12 ++++++++++-
 block/qcow2-cluster.c  | 35 ++++++++++++++++++++++++--------
 block/qcow2-refcount.c | 54 +++++++++++++++++++++++++++++++++++++++-----------
 block/qcow2-snapshot.c | 22 +++++++++++++++-----
 block/qcow2.c          | 41 ++++++++++++++++++++++++++++++--------
 block/qed-check.c      |  7 +++++--
 block/qed.c            |  6 +++++-
 block/raw-posix.c      |  6 +++++-
 block/rbd.c            |  7 +++++--
 block/vdi.c            | 24 +++++++++++++++++-----
 block/vhdx-log.c       |  6 +++++-
 block/vhdx.c           | 12 +++++++++--
 block/vmdk.c           | 12 +++++++++--
 block/vpc.c            |  6 +++++-
 block/win32-aio.c      |  6 +++++-
 include/block/block.h  |  1 +
 include/qemu/osdep.h   |  1 +
 util/oslib-posix.c     | 16 +++++++++------
 util/oslib-win32.c     |  9 +++++++--
 29 files changed, 364 insertions(+), 91 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v3 01/21] block: Introduce qemu_try_blockalign()

[Kevin Wolf]

This function returns NULL instead of aborting when an allocation fails.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c               | 13 +++++++++++++
 include/block/block.h |  1 +
 include/qemu/osdep.h  |  1 +
 util/oslib-posix.c    | 16 ++++++++++------
 util/oslib-win32.c    |  9 +++++++--
 5 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/block.c b/block.c
index 310ea89..6179796 100644
--- a/block.c
+++ b/block.c
@@ -5223,6 +5223,19 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
     return qemu_memalign(bdrv_opt_mem_align(bs), size);
 }
 
+void *qemu_try_blockalign(BlockDriverState *bs, size_t size)
+{
+    size_t align = bdrv_opt_mem_align(bs);
+
+    /* Ensure that NULL is never returned on success */
+    assert(align > 0);
+    if (size == 0) {
+        size = align;
+    }
+
+    return qemu_try_memalign(align, size);
+}
+
 /*
  * Check if all memory in this vector is sector aligned.
  */
diff --git a/include/block/block.h b/include/block/block.h
index faee3aa..c895e92 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -453,6 +453,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
 size_t bdrv_opt_mem_align(BlockDriverState *bs);
 void bdrv_set_guest_block_size(BlockDriverState *bs, int align);
 void *qemu_blockalign(BlockDriverState *bs, size_t size);
+void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
 bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
 
 struct HBitmapIter;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ffb2966..b0e3053 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -95,6 +95,7 @@ typedef signed int              int_fast16_t;
 #define qemu_printf printf
 
 int qemu_daemon(int nochdir, int noclose);
+void *qemu_try_memalign(size_t alignment, size_t size);
 void *qemu_memalign(size_t alignment, size_t size);
 void *qemu_anon_ram_alloc(size_t size);
 void qemu_vfree(void *ptr);
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 8e9c770..16e6200 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -90,7 +90,7 @@ void *qemu_oom_check(void *ptr)
     return ptr;
 }
 
-void *qemu_memalign(size_t alignment, size_t size)
+void *qemu_try_memalign(size_t alignment, size_t size)
 {
     void *ptr;
 
@@ -102,19 +102,23 @@ void *qemu_memalign(size_t alignment, size_t size)
     int ret;
     ret = posix_memalign(&ptr, alignment, size);
     if (ret != 0) {
-        fprintf(stderr, "Failed to allocate %zu B: %s\n",
-                size, strerror(ret));
-        abort();
+        errno = ret;
+        ptr = NULL;
     }
 #elif defined(CONFIG_BSD)
-    ptr = qemu_oom_check(valloc(size));
+    ptr = valloc(size);
 #else
-    ptr = qemu_oom_check(memalign(alignment, size));
+    ptr = memalign(alignment, size);
 #endif
     trace_qemu_memalign(alignment, size, ptr);
     return ptr;
 }
 
+void *qemu_memalign(size_t alignment, size_t size)
+{
+    return qemu_oom_check(qemu_try_memalign(alignment, size));
+}
+
 /* alloc shared memory pages */
 void *qemu_anon_ram_alloc(size_t size)
 {
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index 69552f7..ddc823e 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -46,18 +46,23 @@ void *qemu_oom_check(void *ptr)
     return ptr;
 }
 
-void *qemu_memalign(size_t alignment, size_t size)
+void *qemu_try_memalign(size_t alignment, size_t size)
 {
     void *ptr;
 
     if (!size) {
         abort();
     }
-    ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
     trace_qemu_memalign(alignment, size, ptr);
     return ptr;
 }
 
+void *qemu_memalign(size_t alignment, size_t size)
+{
+    return qemu_oom_check(qemu_try_memalign(alignment, size));
+}
+
 void *qemu_anon_ram_alloc(size_t size)
 {
     void *ptr;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 01/21] block: Introduce qemu_try_blockalign()

[Eric Blake]

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

On 06/03/2014 07:10 AM, Kevin Wolf wrote:
> This function returns NULL instead of aborting when an allocation fails.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 13 +++++++++++++
>  include/block/block.h |  1 +
>  include/qemu/osdep.h  |  1 +
>  util/oslib-posix.c    | 16 ++++++++++------
>  util/oslib-win32.c    |  9 +++++++--
>  5 files changed, 32 insertions(+), 8 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v3 01/21] block: Introduce qemu_try_blockalign()

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:42 (+0200), Kevin Wolf wrote :
> This function returns NULL instead of aborting when an allocation fails.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c               | 13 +++++++++++++
>  include/block/block.h |  1 +
>  include/qemu/osdep.h  |  1 +
>  util/oslib-posix.c    | 16 ++++++++++------
>  util/oslib-win32.c    |  9 +++++++--
>  5 files changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 310ea89..6179796 100644
> --- a/block.c
> +++ b/block.c
> @@ -5223,6 +5223,19 @@ void *qemu_blockalign(BlockDriverState *bs, size_t size)
>      return qemu_memalign(bdrv_opt_mem_align(bs), size);
>  }
>  
> +void *qemu_try_blockalign(BlockDriverState *bs, size_t size)
> +{
> +    size_t align = bdrv_opt_mem_align(bs);
> +
> +    /* Ensure that NULL is never returned on success */
> +    assert(align > 0);
> +    if (size == 0) {
> +        size = align;
> +    }
> +
> +    return qemu_try_memalign(align, size);
> +}
> +
>  /*
>   * Check if all memory in this vector is sector aligned.
>   */
> diff --git a/include/block/block.h b/include/block/block.h
> index faee3aa..c895e92 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -453,6 +453,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
>  size_t bdrv_opt_mem_align(BlockDriverState *bs);
>  void bdrv_set_guest_block_size(BlockDriverState *bs, int align);
>  void *qemu_blockalign(BlockDriverState *bs, size_t size);
> +void *qemu_try_blockalign(BlockDriverState *bs, size_t size);
>  bool bdrv_qiov_is_aligned(BlockDriverState *bs, QEMUIOVector *qiov);
>  
>  struct HBitmapIter;
> diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
> index ffb2966..b0e3053 100644
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -95,6 +95,7 @@ typedef signed int              int_fast16_t;
>  #define qemu_printf printf
>  
>  int qemu_daemon(int nochdir, int noclose);
> +void *qemu_try_memalign(size_t alignment, size_t size);
>  void *qemu_memalign(size_t alignment, size_t size);
>  void *qemu_anon_ram_alloc(size_t size);
>  void qemu_vfree(void *ptr);
> diff --git a/util/oslib-posix.c b/util/oslib-posix.c
> index 8e9c770..16e6200 100644
> --- a/util/oslib-posix.c
> +++ b/util/oslib-posix.c
> @@ -90,7 +90,7 @@ void *qemu_oom_check(void *ptr)
>      return ptr;
>  }
>  
> -void *qemu_memalign(size_t alignment, size_t size)
> +void *qemu_try_memalign(size_t alignment, size_t size)
>  {
>      void *ptr;
>  
> @@ -102,19 +102,23 @@ void *qemu_memalign(size_t alignment, size_t size)
>      int ret;
>      ret = posix_memalign(&ptr, alignment, size);
>      if (ret != 0) {
> -        fprintf(stderr, "Failed to allocate %zu B: %s\n",
> -                size, strerror(ret));
> -        abort();
> +        errno = ret;
> +        ptr = NULL;
>      }
>  #elif defined(CONFIG_BSD)
> -    ptr = qemu_oom_check(valloc(size));
> +    ptr = valloc(size);
>  #else
> -    ptr = qemu_oom_check(memalign(alignment, size));
> +    ptr = memalign(alignment, size);
>  #endif
>      trace_qemu_memalign(alignment, size, ptr);
>      return ptr;
>  }
>  
> +void *qemu_memalign(size_t alignment, size_t size)
> +{
> +    return qemu_oom_check(qemu_try_memalign(alignment, size));
> +}
> +
>  /* alloc shared memory pages */
>  void *qemu_anon_ram_alloc(size_t size)
>  {
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index 69552f7..ddc823e 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -46,18 +46,23 @@ void *qemu_oom_check(void *ptr)
>      return ptr;
>  }
>  
> -void *qemu_memalign(size_t alignment, size_t size)
> +void *qemu_try_memalign(size_t alignment, size_t size)
>  {
>      void *ptr;
>  
>      if (!size) {
>          abort();
>      }
> -    ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
> +    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
>      trace_qemu_memalign(alignment, size, ptr);
>      return ptr;
>  }
>  
> +void *qemu_memalign(size_t alignment, size_t size)
> +{
> +    return qemu_oom_check(qemu_try_memalign(alignment, size));
> +}
> +
>  void *qemu_anon_ram_alloc(size_t size)
>  {
>      void *ptr;
> -- 
> 1.8.3.1
> 

Definitively simpler to use.
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 02/21] block: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses bounce buffer allocations in block.c. While at it,
convert bdrv_commit() from plain g_malloc() to qemu_try_blockalign().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/block.c b/block.c
index 6179796..2a3c4c5 100644
--- a/block.c
+++ b/block.c
@@ -2299,7 +2299,14 @@ int bdrv_commit(BlockDriverState *bs)
     }
 
     total_sectors = length >> BDRV_SECTOR_BITS;
-    buf = g_malloc(COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
+
+    /* qemu_try_blockalign() for bs will choose an alignment that works for
+     * bs->backing_hd as well, so no need to compare the alignment manually. */
+    buf = qemu_try_blockalign(bs, COMMIT_BUF_SECTORS * BDRV_SECTOR_SIZE);
+    if (buf == NULL) {
+        ret = -ENOMEM;
+        goto ro_cleanup;
+    }
 
     for (sector = 0; sector < total_sectors; sector += n) {
         ret = bdrv_is_allocated(bs, sector, COMMIT_BUF_SECTORS, &n);
@@ -2337,7 +2344,7 @@ int bdrv_commit(BlockDriverState *bs)
 
     ret = 0;
 ro_cleanup:
-    g_free(buf);
+    qemu_vfree(buf);
 
     if (ro) {
         /* ignoring error return here */
@@ -2996,7 +3003,12 @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BlockDriverState *bs,
                                    cluster_sector_num, cluster_nb_sectors);
 
     iov.iov_len = cluster_nb_sectors * BDRV_SECTOR_SIZE;
-    iov.iov_base = bounce_buffer = qemu_blockalign(bs, iov.iov_len);
+    iov.iov_base = bounce_buffer = qemu_try_blockalign(bs, iov.iov_len);
+    if (bounce_buffer == NULL) {
+        ret = -ENOMEM;
+        goto err;
+    }
+
     qemu_iovec_init_external(&bounce_qiov, &iov, 1);
 
     ret = drv->bdrv_co_readv(bs, cluster_sector_num, cluster_nb_sectors,
@@ -3268,7 +3280,11 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
             /* Fall back to bounce buffer if write zeroes is unsupported */
             iov.iov_len = num * BDRV_SECTOR_SIZE;
             if (iov.iov_base == NULL) {
-                iov.iov_base = qemu_blockalign(bs, num * BDRV_SECTOR_SIZE);
+                iov.iov_base = qemu_try_blockalign(bs, num * BDRV_SECTOR_SIZE);
+                if (iov.iov_base == NULL) {
+                    ret = -ENOMEM;
+                    goto fail;
+                }
                 memset(iov.iov_base, 0, num * BDRV_SECTOR_SIZE);
             }
             qemu_iovec_init_external(&qiov, &iov, 1);
@@ -3288,6 +3304,7 @@ static int coroutine_fn bdrv_co_do_write_zeroes(BlockDriverState *bs,
         nb_sectors -= num;
     }
 
+fail:
     qemu_vfree(iov.iov_base);
     return ret;
 }
@@ -4597,8 +4614,9 @@ static void bdrv_aio_bh_cb(void *opaque)
 {
     BlockDriverAIOCBSync *acb = opaque;
 
-    if (!acb->is_write)
+    if (!acb->is_write && acb->ret >= 0) {
         qemu_iovec_from_buf(acb->qiov, 0, acb->bounce, acb->qiov->size);
+    }
     qemu_vfree(acb->bounce);
     acb->common.cb(acb->common.opaque, acb->ret);
     qemu_bh_delete(acb->bh);
@@ -4620,10 +4638,12 @@ static BlockDriverAIOCB *bdrv_aio_rw_vector(BlockDriverState *bs,
     acb = qemu_aio_get(&bdrv_em_aiocb_info, bs, cb, opaque);
     acb->is_write = is_write;
     acb->qiov = qiov;
-    acb->bounce = qemu_blockalign(bs, qiov->size);
+    acb->bounce = qemu_try_blockalign(bs, qiov->size);
     acb->bh = qemu_bh_new(bdrv_aio_bh_cb, acb);
 
-    if (is_write) {
+    if (acb->bounce == NULL) {
+        acb->ret = -ENOMEM;
+    } else if (is_write) {
         qemu_iovec_to_buf(acb->qiov, 0, acb->bounce, qiov->size);
         acb->ret = bs->drv->bdrv_write(bs, sector_num, acb->bounce, nb_sectors);
     } else {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 03/21] bochs: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the bochs block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/bochs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/bochs.c b/block/bochs.c
index eba23df..6674b27 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -131,7 +131,11 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
         return -EFBIG;
     }
 
-    s->catalog_bitmap = g_malloc(s->catalog_size * 4);
+    s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+    if (s->catalog_size && s->catalog_bitmap == NULL) {
+        error_setg(errp, "Could not allocate memory for catalog");
+        return -ENOMEM;
+    }
 
     ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
                      s->catalog_size * 4);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 04/21] cloop: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the cloop block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/cloop.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/block/cloop.c b/block/cloop.c
index 8457737..f328be0 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -116,7 +116,12 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
                    "try increasing block size");
         return -EINVAL;
     }
-    s->offsets = g_malloc(offsets_size);
+
+    s->offsets = g_try_malloc(offsets_size);
+    if (s->offsets == NULL) {
+        error_setg(errp, "Could not allocate offsets table");
+        return -ENOMEM;
+    }
 
     ret = bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size);
     if (ret < 0) {
@@ -158,8 +163,20 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* initialize zlib engine */
-    s->compressed_block = g_malloc(max_compressed_block_size + 1);
-    s->uncompressed_block = g_malloc(s->block_size);
+    s->compressed_block = g_try_malloc(max_compressed_block_size + 1);
+    if (s->compressed_block == NULL) {
+        error_setg(errp, "Could not allocate compressed_block");
+        ret = -ENOMEM;
+        goto fail;
+    }
+
+    s->uncompressed_block = g_try_malloc(s->block_size);
+    if (s->uncompressed_block == NULL) {
+        error_setg(errp, "Could not allocate uncompressed_block");
+        ret = -ENOMEM;
+        goto fail;
+    }
+
     if (inflateInit(&s->zstream) != Z_OK) {
         ret = -EINVAL;
         goto fail;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 05/21] curl: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the curl block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/curl.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/block/curl.c b/block/curl.c
index f491b0b..ae996e2 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -604,7 +604,13 @@ static void curl_readv_bh_cb(void *p)
     state->buf_start = start;
     state->buf_len = acb->end + s->readahead_size;
     end = MIN(start + state->buf_len, s->len) - 1;
-    state->orig_buf = g_malloc(state->buf_len);
+    state->orig_buf = g_try_malloc(state->buf_len);
+    if (state->buf_len && state->orig_buf == NULL) {
+        curl_clean_state(state);
+        acb->common.cb(acb->common.opaque, -ENOMEM);
+        qemu_aio_release(acb);
+        return;
+    }
     state->acb[0] = acb;
 
     snprintf(state->range, 127, "%zd-%zd", start, end);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 06/21] dmg: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the dmg block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/dmg.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index 1e153cd..e455886 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -284,8 +284,15 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* initialize zlib engine */
-    s->compressed_chunk = g_malloc(max_compressed_size + 1);
-    s->uncompressed_chunk = g_malloc(512 * max_sectors_per_chunk);
+    s->compressed_chunk = qemu_try_blockalign(bs->file,
+                                              max_compressed_size + 1);
+    s->uncompressed_chunk = qemu_try_blockalign(bs->file,
+                                                512 * max_sectors_per_chunk);
+    if (s->compressed_chunk == NULL || s->uncompressed_chunk == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
+
     if (inflateInit(&s->zstream) != Z_OK) {
         ret = -EINVAL;
         goto fail;
@@ -302,8 +309,8 @@ fail:
     g_free(s->lengths);
     g_free(s->sectors);
     g_free(s->sectorcounts);
-    g_free(s->compressed_chunk);
-    g_free(s->uncompressed_chunk);
+    qemu_vfree(s->compressed_chunk);
+    qemu_vfree(s->uncompressed_chunk);
     return ret;
 }
 
@@ -426,8 +433,8 @@ static void dmg_close(BlockDriverState *bs)
     g_free(s->lengths);
     g_free(s->sectors);
     g_free(s->sectorcounts);
-    g_free(s->compressed_chunk);
-    g_free(s->uncompressed_chunk);
+    qemu_vfree(s->compressed_chunk);
+    qemu_vfree(s->uncompressed_chunk);
 
     inflateEnd(&s->zstream);
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 07/21] iscsi: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the iscsi block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
 block/iscsi.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/iscsi.c b/block/iscsi.c
index 3892cc5..b3e9bdd 100644
--- a/block/iscsi.c
+++ b/block/iscsi.c
@@ -335,7 +335,10 @@ static int coroutine_fn iscsi_co_writev(BlockDriverState *bs,
         data = iov->iov[0].iov_base;
     } else {
         size_t size = MIN(nb_sectors * BDRV_SECTOR_SIZE, iov->size);
-        buf = g_malloc(size);
+        buf = g_try_malloc(size);
+        if (size && buf == NULL) {
+            return -ENOMEM;
+        }
         qemu_iovec_to_buf(iov, 0, buf, size);
         data = buf;
     }
@@ -721,7 +724,12 @@ static BlockDriverAIOCB *iscsi_aio_ioctl(BlockDriverState *bs,
 #else
             struct iovec *iov = (struct iovec *)acb->ioh->dxferp;
 
-            acb->buf = g_malloc(acb->ioh->dxfer_len);
+            acb->buf = g_try_malloc(acb->ioh->dxfer_len);
+            if (acb->ioh->dxfer_len && acb->buf == NULL) {
+                free(acb->task);
+                qemu_aio_release(acb);
+                return NULL;
+            }
             data.data = acb->buf;
             data.size = iov_to_buf(iov, acb->ioh->iovec_count, 0,
                                    acb->buf, acb->ioh->dxfer_len);
@@ -902,7 +910,10 @@ coroutine_fn iscsi_co_write_zeroes(BlockDriverState *bs, int64_t sector_num,
     nb_blocks = sector_qemu2lun(nb_sectors, iscsilun);
 
     if (iscsilun->zeroblock == NULL) {
-        iscsilun->zeroblock = g_malloc0(iscsilun->block_size);
+        iscsilun->zeroblock = g_try_malloc0(iscsilun->block_size);
+        if (iscsilun->zeroblock == NULL) {
+            return -ENOMEM;
+        }
     }
 
     iscsi_co_init_iscsitask(iscsilun, &iTask);
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 08/21] nfs: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the nfs block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/nfs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/nfs.c b/block/nfs.c
index 539bd95..b37316f 100644
--- a/block/nfs.c
+++ b/block/nfs.c
@@ -165,7 +165,11 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
 
     nfs_co_init_task(client, &task);
 
-    buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+    buf = g_try_malloc(nb_sectors * BDRV_SECTOR_SIZE);
+    if (nb_sectors && buf == NULL) {
+        return -ENOMEM;
+    }
+
     qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
 
     if (nfs_pwrite_async(client->context, client->fh,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 08/21] nfs: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:49 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the nfs block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/nfs.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/block/nfs.c b/block/nfs.c
> index 539bd95..b37316f 100644
> --- a/block/nfs.c
> +++ b/block/nfs.c
> @@ -165,7 +165,11 @@ static int coroutine_fn nfs_co_writev(BlockDriverState *bs,
>  
>      nfs_co_init_task(client, &task);
>  
> -    buf = g_malloc(nb_sectors * BDRV_SECTOR_SIZE);
> +    buf = g_try_malloc(nb_sectors * BDRV_SECTOR_SIZE);
> +    if (nb_sectors && buf == NULL) {
> +        return -ENOMEM;
> +    }
> +
>      qemu_iovec_to_buf(iov, 0, buf, nb_sectors * BDRV_SECTOR_SIZE);
>  
>      if (nfs_pwrite_async(client->context, client->fh,
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 09/21] parallels: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the parallels block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 block/parallels.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index 1a5bd35..7325678 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -105,7 +105,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EFBIG;
         goto fail;
     }
-    s->catalog_bitmap = g_malloc(s->catalog_size * 4);
+    s->catalog_bitmap = g_try_malloc(s->catalog_size * 4);
+    if (s->catalog_size && s->catalog_bitmap == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     ret = bdrv_pread(bs->file, 64, s->catalog_bitmap, s->catalog_size * 4);
     if (ret < 0) {
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 10/21] qcow1: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the qcow1 block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow.c | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/block/qcow.c b/block/qcow.c
index 7fd57d7..31db585 100644
--- a/block/qcow.c
+++ b/block/qcow.c
@@ -182,7 +182,12 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     s->l1_table_offset = header.l1_table_offset;
-    s->l1_table = g_malloc(s->l1_size * sizeof(uint64_t));
+    s->l1_table = g_try_malloc(s->l1_size * sizeof(uint64_t));
+    if (s->l1_table == NULL) {
+        error_setg(errp, "Could not allocate memory for L1 table");
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
                s->l1_size * sizeof(uint64_t));
@@ -193,8 +198,16 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
     for(i = 0;i < s->l1_size; i++) {
         be64_to_cpus(&s->l1_table[i]);
     }
-    /* alloc L2 cache */
-    s->l2_cache = g_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t));
+
+    /* alloc L2 cache (max. 64k * 16 * 8 = 8 MB) */
+    s->l2_cache =
+        qemu_try_blockalign(bs->file,
+                            s->l2_size * L2_CACHE_SIZE * sizeof(uint64_t));
+    if (s->l2_cache == NULL) {
+        error_setg(errp, "Could not allocate L2 table cache");
+        ret = -ENOMEM;
+        goto fail;
+    }
     s->cluster_cache = g_malloc(s->cluster_size);
     s->cluster_data = g_malloc(s->cluster_size);
     s->cluster_cache_offset = -1;
@@ -226,7 +239,7 @@ static int qcow_open(BlockDriverState *bs, QDict *options, int flags,
 
  fail:
     g_free(s->l1_table);
-    g_free(s->l2_cache);
+    qemu_vfree(s->l2_cache);
     g_free(s->cluster_cache);
     g_free(s->cluster_data);
     return ret;
@@ -517,7 +530,10 @@ static coroutine_fn int qcow_co_readv(BlockDriverState *bs, int64_t sector_num,
     void *orig_buf;
 
     if (qiov->niov > 1) {
-        buf = orig_buf = qemu_blockalign(bs, qiov->size);
+        buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
+        if (buf == NULL) {
+            return -ENOMEM;
+        }
     } else {
         orig_buf = NULL;
         buf = (uint8_t *)qiov->iov->iov_base;
@@ -619,7 +635,10 @@ static coroutine_fn int qcow_co_writev(BlockDriverState *bs, int64_t sector_num,
     s->cluster_cache_offset = -1; /* disable compressed cache */
 
     if (qiov->niov > 1) {
-        buf = orig_buf = qemu_blockalign(bs, qiov->size);
+        buf = orig_buf = qemu_try_blockalign(bs, qiov->size);
+        if (buf == NULL) {
+            return -ENOMEM;
+        }
         qemu_iovec_to_buf(qiov, 0, buf, qiov->size);
     } else {
         orig_buf = NULL;
@@ -685,7 +704,7 @@ static void qcow_close(BlockDriverState *bs)
     BDRVQcowState *s = bs->opaque;
 
     g_free(s->l1_table);
-    g_free(s->l2_cache);
+    qemu_vfree(s->l2_cache);
     g_free(s->cluster_cache);
     g_free(s->cluster_data);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 11/21] qcow2: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the qcow2 block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cache.c    | 12 +++++++++++-
 block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
 block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
 block/qcow2-snapshot.c | 22 +++++++++++++++++-----
 block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
 5 files changed, 126 insertions(+), 32 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 8ecbb5b..351bc01 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -53,10 +53,20 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
     c->entries = g_malloc0(sizeof(*c->entries) * num_tables);
 
     for (i = 0; i < c->size; i++) {
-        c->entries[i].table = qemu_blockalign(bs, s->cluster_size);
+        c->entries[i].table = qemu_try_blockalign(bs, s->cluster_size);
+        if (c->entries[i].table == NULL) {
+            goto fail;
+        }
     }
 
     return c;
+
+fail:
+    for (i = 0; i < c->size; i++) {
+        qemu_vfree(c->entries[i].table);
+    }
+    g_free(c);
+    return NULL;
 }
 
 int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c)
diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 4208dc0..d391c5a 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -72,14 +72,19 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
 #endif
 
     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
-    new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
+    new_l1_table = qemu_try_blockalign(bs, align_offset(new_l1_size2, 512));
+    if (new_l1_table == NULL) {
+        return -ENOMEM;
+    }
+    memset(new_l1_table, 0, align_offset(new_l1_size2, 512));
+
     memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
 
     /* write new table (align to cluster) */
     BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);
     new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2);
     if (new_l1_table_offset < 0) {
-        g_free(new_l1_table);
+        qemu_vfree(new_l1_table);
         return new_l1_table_offset;
     }
 
@@ -113,7 +118,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
     if (ret < 0) {
         goto fail;
     }
-    g_free(s->l1_table);
+    qemu_vfree(s->l1_table);
     old_l1_table_offset = s->l1_table_offset;
     s->l1_table_offset = new_l1_table_offset;
     s->l1_table = new_l1_table;
@@ -123,7 +128,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
                         QCOW2_DISCARD_OTHER);
     return 0;
  fail:
-    g_free(new_l1_table);
+    qemu_vfree(new_l1_table);
     qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2,
                         QCOW2_DISCARD_OTHER);
     return ret;
@@ -372,7 +377,10 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
     }
 
     iov.iov_len = n * BDRV_SECTOR_SIZE;
-    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
+    iov.iov_base = qemu_try_blockalign(bs, iov.iov_len);
+    if (iov.iov_base == NULL) {
+        return -ENOMEM;
+    }
 
     qemu_iovec_init_external(&qiov, &iov, 1);
 
@@ -702,7 +710,11 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
     trace_qcow2_cluster_link_l2(qemu_coroutine_self(), m->nb_clusters);
     assert(m->nb_clusters > 0);
 
-    old_cluster = g_malloc(m->nb_clusters * sizeof(uint64_t));
+    old_cluster = g_try_malloc(m->nb_clusters * sizeof(uint64_t));
+    if (old_cluster == NULL) {
+        ret = -ENOMEM;
+        goto err;
+    }
 
     /* copy content of unmodified sectors */
     ret = perform_cow(bs, m, &m->cow_start);
@@ -1562,7 +1574,10 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
     if (!is_active_l1) {
         /* inactive L2 tables require a buffer to be stored in when loading
          * them from disk */
-        l2_table = qemu_blockalign(bs, s->cluster_size);
+        l2_table = qemu_try_blockalign(bs, s->cluster_size);
+        if (l2_table == NULL) {
+            return -ENOMEM;
+        }
     }
 
     for (i = 0; i < l1_size; i++) {
@@ -1740,7 +1755,11 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs)
 
     nb_clusters = size_to_clusters(s, bs->file->total_sectors *
                                    BDRV_SECTOR_SIZE);
-    expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
+    expanded_clusters = g_try_malloc0((nb_clusters + 7) / 8);
+    if (expanded_clusters == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
                                      &expanded_clusters, &nb_clusters);
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9507aef..a234c7a 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -45,8 +45,12 @@ int qcow2_refcount_init(BlockDriverState *bs)
 
     assert(s->refcount_table_size <= INT_MAX / sizeof(uint64_t));
     refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
-    s->refcount_table = g_malloc(refcount_table_size2);
+    s->refcount_table = g_try_malloc(refcount_table_size2);
+
     if (s->refcount_table_size > 0) {
+        if (s->refcount_table == NULL) {
+            goto fail;
+        }
         BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_LOAD);
         ret = bdrv_pread(bs->file, s->refcount_table_offset,
                          s->refcount_table, refcount_table_size2);
@@ -343,8 +347,14 @@ static int alloc_refcount_block(BlockDriverState *bs,
     uint64_t meta_offset = (blocks_used * refcount_block_clusters) *
         s->cluster_size;
     uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size;
-    uint16_t *new_blocks = g_malloc0(blocks_clusters * s->cluster_size);
-    uint64_t *new_table = g_malloc0(table_size * sizeof(uint64_t));
+    uint64_t *new_table = g_try_malloc0(table_size * sizeof(uint64_t));
+    uint16_t *new_blocks = g_try_malloc0(blocks_clusters * s->cluster_size);
+
+    assert(table_size > 0 && blocks_clusters > 0);
+    if (new_table == NULL || new_blocks == NULL) {
+        ret = -ENOMEM;
+        goto fail_table;
+    }
 
     /* Fill the new refcount table */
     memcpy(new_table, s->refcount_table,
@@ -423,6 +433,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
     return -EAGAIN;
 
 fail_table:
+    g_free(new_blocks);
     g_free(new_table);
 fail_block:
     if (*refcount_block != NULL) {
@@ -846,7 +857,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     int64_t l1_table_offset, int l1_size, int addend)
 {
     BDRVQcowState *s = bs->opaque;
-    uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, l1_allocated;
+    uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2;
+    bool l1_allocated = false;
     int64_t old_offset, old_l2_offset;
     int i, j, l1_modified = 0, nb_csectors, refcount;
     int ret;
@@ -861,8 +873,12 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
      * l1_table_offset when it is the current s->l1_table_offset! Be careful
      * when changing this! */
     if (l1_table_offset != s->l1_table_offset) {
-        l1_table = g_malloc0(align_offset(l1_size2, 512));
-        l1_allocated = 1;
+        l1_table = g_try_malloc0(align_offset(l1_size2, 512));
+        if (l1_size2 && l1_table == NULL) {
+            ret = -ENOMEM;
+            goto fail;
+        }
+        l1_allocated = true;
 
         ret = bdrv_pread(bs->file, l1_table_offset, l1_table, l1_size2);
         if (ret < 0) {
@@ -874,7 +890,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
     } else {
         assert(l1_size == s->l1_size);
         l1_table = s->l1_table;
-        l1_allocated = 0;
+        l1_allocated = false;
     }
 
     for(i = 0; i < l1_size; i++) {
@@ -1196,7 +1212,11 @@ static int check_refcounts_l1(BlockDriverState *bs,
     if (l1_size2 == 0) {
         l1_table = NULL;
     } else {
-        l1_table = g_malloc(l1_size2);
+        l1_table = g_try_malloc(l1_size2);
+        if (l1_table == NULL) {
+            ret = -ENOMEM;
+            goto fail;
+        }
         if (bdrv_pread(bs->file, l1_table_offset,
                        l1_table, l1_size2) != l1_size2)
             goto fail;
@@ -1500,7 +1520,11 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
         return -EFBIG;
     }
 
-    refcount_table = g_malloc0(nb_clusters * sizeof(uint16_t));
+    refcount_table = g_try_malloc0(nb_clusters * sizeof(uint16_t));
+    if (nb_clusters && refcount_table == NULL) {
+        res->check_errors++;
+        return -ENOMEM;
+    }
 
     res->bfi.total_clusters =
         size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE);
@@ -1752,9 +1776,13 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
             uint64_t l1_ofs = s->snapshots[i].l1_table_offset;
             uint32_t l1_sz  = s->snapshots[i].l1_size;
             uint64_t l1_sz2 = l1_sz * sizeof(uint64_t);
-            uint64_t *l1 = g_malloc(l1_sz2);
+            uint64_t *l1 = g_try_malloc(l1_sz2);
             int ret;
 
+            if (l1_sz2 && l1 == NULL) {
+                return -ENOMEM;
+            }
+
             ret = bdrv_pread(bs->file, l1_ofs, l1, l1_sz2);
             if (ret < 0) {
                 g_free(l1);
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 0aa9def..07e8b73 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -381,7 +381,12 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     sn->l1_table_offset = l1_table_offset;
     sn->l1_size = s->l1_size;
 
-    l1_table = g_malloc(s->l1_size * sizeof(uint64_t));
+    l1_table = g_try_malloc(s->l1_size * sizeof(uint64_t));
+    if (s->l1_size && l1_table == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
+
     for(i = 0; i < s->l1_size; i++) {
         l1_table[i] = cpu_to_be64(s->l1_table[i]);
     }
@@ -499,7 +504,11 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
      * Decrease the refcount referenced by the old one only when the L1
      * table is overwritten.
      */
-    sn_l1_table = g_malloc0(cur_l1_bytes);
+    sn_l1_table = g_try_malloc0(cur_l1_bytes);
+    if (cur_l1_bytes && sn_l1_table == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     ret = bdrv_pread(bs->file, sn->l1_table_offset, sn_l1_table, sn_l1_bytes);
     if (ret < 0) {
@@ -698,17 +707,20 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
         return -EFBIG;
     }
     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
-    new_l1_table = g_malloc0(align_offset(new_l1_bytes, 512));
+    new_l1_table = qemu_try_blockalign(bs, align_offset(new_l1_bytes, 512));
+    if (new_l1_table == NULL) {
+        return -ENOMEM;
+    }
 
     ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
     if (ret < 0) {
         error_setg(errp, "Failed to read l1 table for snapshot");
-        g_free(new_l1_table);
+        qemu_vfree(new_l1_table);
         return ret;
     }
 
     /* Switch the L1 table */
-    g_free(s->l1_table);
+    qemu_vfree(s->l1_table);
 
     s->l1_size = sn->l1_size;
     s->l1_table_offset = sn->l1_table_offset;
diff --git a/block/qcow2.c b/block/qcow2.c
index a54d2ba..0b07319 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -676,8 +676,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
 
     if (s->l1_size > 0) {
-        s->l1_table = g_malloc0(
+        s->l1_table = qemu_try_blockalign(bs->file,
             align_offset(s->l1_size * sizeof(uint64_t), 512));
+        if (s->l1_table == NULL) {
+            error_setg(errp, "Could not allocate L1 table");
+            ret = -ENOMEM;
+            goto fail;
+        }
         ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
                          s->l1_size * sizeof(uint64_t));
         if (ret < 0) {
@@ -692,11 +697,22 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     /* alloc L2 table/refcount block cache */
     s->l2_table_cache = qcow2_cache_create(bs, L2_CACHE_SIZE);
     s->refcount_block_cache = qcow2_cache_create(bs, REFCOUNT_CACHE_SIZE);
+    if (s->l2_table_cache == NULL || s->refcount_block_cache == NULL) {
+        error_setg(errp, "Could not allocate metadata caches");
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     s->cluster_cache = g_malloc(s->cluster_size);
     /* one more sector for decompressed data alignment */
-    s->cluster_data = qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size
-                                  + 512);
+    s->cluster_data = qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
+                                              * s->cluster_size + 512);
+    if (s->cluster_data == NULL) {
+        error_setg(errp, "Could not allocate temporary cluster buffer");
+        ret = -ENOMEM;
+        goto fail;
+    }
+
     s->cluster_cache_offset = -1;
     s->flags = flags;
 
@@ -840,7 +856,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     cleanup_unknown_header_ext(bs);
     qcow2_free_snapshots(bs);
     qcow2_refcount_close(bs);
-    g_free(s->l1_table);
+    qemu_vfree(s->l1_table);
     /* else pre-write overlap checks in cache_destroy may crash */
     s->l1_table = NULL;
     if (s->l2_table_cache) {
@@ -1063,7 +1079,12 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
                  */
                 if (!cluster_data) {
                     cluster_data =
-                        qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
+                        qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
+                                                * s->cluster_size);
+                    if (cluster_data == NULL) {
+                        ret = -ENOMEM;
+                        goto fail;
+                    }
                 }
 
                 assert(cur_nr_sectors <=
@@ -1163,8 +1184,12 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
 
         if (s->crypt_method) {
             if (!cluster_data) {
-                cluster_data = qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS *
-                                                 s->cluster_size);
+                cluster_data = qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
+                                                       * s->cluster_size);
+                if (cluster_data == NULL) {
+                    ret = -ENOMEM;
+                    goto fail;
+                }
             }
 
             assert(hd_qiov.size <=
@@ -1251,7 +1276,7 @@ fail:
 static void qcow2_close(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
-    g_free(s->l1_table);
+    qemu_vfree(s->l1_table);
     /* else pre-write overlap checks in cache_destroy may crash */
     s->l1_table = NULL;
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 11/21] qcow2: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:52 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the qcow2 block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/qcow2-cache.c    | 12 +++++++++++-
>  block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
>  block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
>  block/qcow2-snapshot.c | 22 +++++++++++++++++-----
>  block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
>  5 files changed, 126 insertions(+), 32 deletions(-)
> 
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index 8ecbb5b..351bc01 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
> @@ -53,10 +53,20 @@ Qcow2Cache *qcow2_cache_create(BlockDriverState *bs, int num_tables)
>      c->entries = g_malloc0(sizeof(*c->entries) * num_tables);
Do we care to free c->entries on failure ?

>  
>      for (i = 0; i < c->size; i++) {
> -        c->entries[i].table = qemu_blockalign(bs, s->cluster_size);
> +        c->entries[i].table = qemu_try_blockalign(bs, s->cluster_size);
> +        if (c->entries[i].table == NULL) {
> +            goto fail;
> +        }
>      }
>  
>      return c;
> +
> +fail:
> +    for (i = 0; i < c->size; i++) {
> +        qemu_vfree(c->entries[i].table);
> +    }
> +    g_free(c);
> +    return NULL;
>  }
>  
>  int qcow2_cache_destroy(BlockDriverState* bs, Qcow2Cache *c)
> diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
> index 4208dc0..d391c5a 100644
> --- a/block/qcow2-cluster.c
> +++ b/block/qcow2-cluster.c
> @@ -72,14 +72,19 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>  #endif
>  
>      new_l1_size2 = sizeof(uint64_t) * new_l1_size;
> -    new_l1_table = g_malloc0(align_offset(new_l1_size2, 512));
> +    new_l1_table = qemu_try_blockalign(bs, align_offset(new_l1_size2, 512));
> +    if (new_l1_table == NULL) {
> +        return -ENOMEM;
> +    }
> +    memset(new_l1_table, 0, align_offset(new_l1_size2, 512));
> +
>      memcpy(new_l1_table, s->l1_table, s->l1_size * sizeof(uint64_t));
>  
>      /* write new table (align to cluster) */
>      BLKDBG_EVENT(bs->file, BLKDBG_L1_GROW_ALLOC_TABLE);
>      new_l1_table_offset = qcow2_alloc_clusters(bs, new_l1_size2);
>      if (new_l1_table_offset < 0) {
> -        g_free(new_l1_table);
> +        qemu_vfree(new_l1_table);
>          return new_l1_table_offset;
>      }
>  
> @@ -113,7 +118,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>      if (ret < 0) {
>          goto fail;
>      }
> -    g_free(s->l1_table);
> +    qemu_vfree(s->l1_table);
>      old_l1_table_offset = s->l1_table_offset;
>      s->l1_table_offset = new_l1_table_offset;
>      s->l1_table = new_l1_table;
> @@ -123,7 +128,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
>                          QCOW2_DISCARD_OTHER);
>      return 0;
>   fail:
> -    g_free(new_l1_table);
> +    qemu_vfree(new_l1_table);
>      qcow2_free_clusters(bs, new_l1_table_offset, new_l1_size2,
>                          QCOW2_DISCARD_OTHER);
>      return ret;
> @@ -372,7 +377,10 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
>      }
>  
>      iov.iov_len = n * BDRV_SECTOR_SIZE;
> -    iov.iov_base = qemu_blockalign(bs, iov.iov_len);
> +    iov.iov_base = qemu_try_blockalign(bs, iov.iov_len);
> +    if (iov.iov_base == NULL) {
> +        return -ENOMEM;
> +    }
>  
>      qemu_iovec_init_external(&qiov, &iov, 1);
>  
> @@ -702,7 +710,11 @@ int qcow2_alloc_cluster_link_l2(BlockDriverState *bs, QCowL2Meta *m)
>      trace_qcow2_cluster_link_l2(qemu_coroutine_self(), m->nb_clusters);
>      assert(m->nb_clusters > 0);
>  
> -    old_cluster = g_malloc(m->nb_clusters * sizeof(uint64_t));
> +    old_cluster = g_try_malloc(m->nb_clusters * sizeof(uint64_t));
> +    if (old_cluster == NULL) {
> +        ret = -ENOMEM;
> +        goto err;
> +    }
>  
>      /* copy content of unmodified sectors */
>      ret = perform_cow(bs, m, &m->cow_start);
> @@ -1562,7 +1574,10 @@ static int expand_zero_clusters_in_l1(BlockDriverState *bs, uint64_t *l1_table,
>      if (!is_active_l1) {
>          /* inactive L2 tables require a buffer to be stored in when loading
>           * them from disk */
> -        l2_table = qemu_blockalign(bs, s->cluster_size);
> +        l2_table = qemu_try_blockalign(bs, s->cluster_size);
> +        if (l2_table == NULL) {
> +            return -ENOMEM;
> +        }
>      }
>  
>      for (i = 0; i < l1_size; i++) {
> @@ -1740,7 +1755,11 @@ int qcow2_expand_zero_clusters(BlockDriverState *bs)
>  
>      nb_clusters = size_to_clusters(s, bs->file->total_sectors *
>                                     BDRV_SECTOR_SIZE);
> -    expanded_clusters = g_malloc0((nb_clusters + 7) / 8);
> +    expanded_clusters = g_try_malloc0((nb_clusters + 7) / 8);
> +    if (expanded_clusters == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
>  
>      ret = expand_zero_clusters_in_l1(bs, s->l1_table, s->l1_size,
>                                       &expanded_clusters, &nb_clusters);
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index 9507aef..a234c7a 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -45,8 +45,12 @@ int qcow2_refcount_init(BlockDriverState *bs)
>  
>      assert(s->refcount_table_size <= INT_MAX / sizeof(uint64_t));
>      refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
> -    s->refcount_table = g_malloc(refcount_table_size2);
> +    s->refcount_table = g_try_malloc(refcount_table_size2);
> +
>      if (s->refcount_table_size > 0) {
> +        if (s->refcount_table == NULL) {
> +            goto fail;
> +        }
>          BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_LOAD);
>          ret = bdrv_pread(bs->file, s->refcount_table_offset,
>                           s->refcount_table, refcount_table_size2);
> @@ -343,8 +347,14 @@ static int alloc_refcount_block(BlockDriverState *bs,
>      uint64_t meta_offset = (blocks_used * refcount_block_clusters) *
>          s->cluster_size;
>      uint64_t table_offset = meta_offset + blocks_clusters * s->cluster_size;
> -    uint16_t *new_blocks = g_malloc0(blocks_clusters * s->cluster_size);
> -    uint64_t *new_table = g_malloc0(table_size * sizeof(uint64_t));
> +    uint64_t *new_table = g_try_malloc0(table_size * sizeof(uint64_t));
> +    uint16_t *new_blocks = g_try_malloc0(blocks_clusters * s->cluster_size);
> +
> +    assert(table_size > 0 && blocks_clusters > 0);
> +    if (new_table == NULL || new_blocks == NULL) {
> +        ret = -ENOMEM;
> +        goto fail_table;
> +    }
>  
>      /* Fill the new refcount table */
>      memcpy(new_table, s->refcount_table,
> @@ -423,6 +433,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
>      return -EAGAIN;
>  
>  fail_table:
> +    g_free(new_blocks);
>      g_free(new_table);
>  fail_block:
>      if (*refcount_block != NULL) {
> @@ -846,7 +857,8 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>      int64_t l1_table_offset, int l1_size, int addend)
>  {
>      BDRVQcowState *s = bs->opaque;
> -    uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2, l1_allocated;
> +    uint64_t *l1_table, *l2_table, l2_offset, offset, l1_size2;
> +    bool l1_allocated = false;
>      int64_t old_offset, old_l2_offset;
>      int i, j, l1_modified = 0, nb_csectors, refcount;
>      int ret;
> @@ -861,8 +873,12 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>       * l1_table_offset when it is the current s->l1_table_offset! Be careful
>       * when changing this! */
>      if (l1_table_offset != s->l1_table_offset) {
> -        l1_table = g_malloc0(align_offset(l1_size2, 512));
> -        l1_allocated = 1;
> +        l1_table = g_try_malloc0(align_offset(l1_size2, 512));
> +        if (l1_size2 && l1_table == NULL) {
> +            ret = -ENOMEM;
> +            goto fail;
> +        }
> +        l1_allocated = true;
>  
>          ret = bdrv_pread(bs->file, l1_table_offset, l1_table, l1_size2);
>          if (ret < 0) {
> @@ -874,7 +890,7 @@ int qcow2_update_snapshot_refcount(BlockDriverState *bs,
>      } else {
>          assert(l1_size == s->l1_size);
>          l1_table = s->l1_table;
> -        l1_allocated = 0;
> +        l1_allocated = false;
>      }
>  
>      for(i = 0; i < l1_size; i++) {
> @@ -1196,7 +1212,11 @@ static int check_refcounts_l1(BlockDriverState *bs,
>      if (l1_size2 == 0) {
>          l1_table = NULL;
>      } else {
> -        l1_table = g_malloc(l1_size2);
> +        l1_table = g_try_malloc(l1_size2);
> +        if (l1_table == NULL) {
> +            ret = -ENOMEM;
> +            goto fail;
> +        }
>          if (bdrv_pread(bs->file, l1_table_offset,
>                         l1_table, l1_size2) != l1_size2)
>              goto fail;
> @@ -1500,7 +1520,11 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
>          return -EFBIG;
>      }
>  
> -    refcount_table = g_malloc0(nb_clusters * sizeof(uint16_t));
> +    refcount_table = g_try_malloc0(nb_clusters * sizeof(uint16_t));
> +    if (nb_clusters && refcount_table == NULL) {
> +        res->check_errors++;
> +        return -ENOMEM;
> +    }
>  
>      res->bfi.total_clusters =
>          size_to_clusters(s, bs->total_sectors * BDRV_SECTOR_SIZE);
> @@ -1752,9 +1776,13 @@ int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
>              uint64_t l1_ofs = s->snapshots[i].l1_table_offset;
>              uint32_t l1_sz  = s->snapshots[i].l1_size;
>              uint64_t l1_sz2 = l1_sz * sizeof(uint64_t);
> -            uint64_t *l1 = g_malloc(l1_sz2);
> +            uint64_t *l1 = g_try_malloc(l1_sz2);
>              int ret;
>  
> +            if (l1_sz2 && l1 == NULL) {
> +                return -ENOMEM;
> +            }
> +
>              ret = bdrv_pread(bs->file, l1_ofs, l1, l1_sz2);
>              if (ret < 0) {
>                  g_free(l1);
> diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
> index 0aa9def..07e8b73 100644
> --- a/block/qcow2-snapshot.c
> +++ b/block/qcow2-snapshot.c
> @@ -381,7 +381,12 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
>      sn->l1_table_offset = l1_table_offset;
>      sn->l1_size = s->l1_size;
>  
> -    l1_table = g_malloc(s->l1_size * sizeof(uint64_t));
> +    l1_table = g_try_malloc(s->l1_size * sizeof(uint64_t));
> +    if (s->l1_size && l1_table == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
> +
>      for(i = 0; i < s->l1_size; i++) {
>          l1_table[i] = cpu_to_be64(s->l1_table[i]);
>      }
> @@ -499,7 +504,11 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char *snapshot_id)
>       * Decrease the refcount referenced by the old one only when the L1
>       * table is overwritten.
>       */
> -    sn_l1_table = g_malloc0(cur_l1_bytes);
> +    sn_l1_table = g_try_malloc0(cur_l1_bytes);
> +    if (cur_l1_bytes && sn_l1_table == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
>  
>      ret = bdrv_pread(bs->file, sn->l1_table_offset, sn_l1_table, sn_l1_bytes);
>      if (ret < 0) {
> @@ -698,17 +707,20 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
>          return -EFBIG;
>      }
>      new_l1_bytes = sn->l1_size * sizeof(uint64_t);
> -    new_l1_table = g_malloc0(align_offset(new_l1_bytes, 512));
> +    new_l1_table = qemu_try_blockalign(bs, align_offset(new_l1_bytes, 512));
> +    if (new_l1_table == NULL) {
> +        return -ENOMEM;
> +    }
>  
>      ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
>      if (ret < 0) {
>          error_setg(errp, "Failed to read l1 table for snapshot");
> -        g_free(new_l1_table);
> +        qemu_vfree(new_l1_table);
>          return ret;
>      }
>  
>      /* Switch the L1 table */
> -    g_free(s->l1_table);
> +    qemu_vfree(s->l1_table);
>  
>      s->l1_size = sn->l1_size;
>      s->l1_table_offset = sn->l1_table_offset;
> diff --git a/block/qcow2.c b/block/qcow2.c
> index a54d2ba..0b07319 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -676,8 +676,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>  
>  
>      if (s->l1_size > 0) {
> -        s->l1_table = g_malloc0(
> +        s->l1_table = qemu_try_blockalign(bs->file,
>              align_offset(s->l1_size * sizeof(uint64_t), 512));
> +        if (s->l1_table == NULL) {
> +            error_setg(errp, "Could not allocate L1 table");
> +            ret = -ENOMEM;
> +            goto fail;
> +        }
>          ret = bdrv_pread(bs->file, s->l1_table_offset, s->l1_table,
>                           s->l1_size * sizeof(uint64_t));
>          if (ret < 0) {
> @@ -692,11 +697,22 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>      /* alloc L2 table/refcount block cache */
>      s->l2_table_cache = qcow2_cache_create(bs, L2_CACHE_SIZE);
>      s->refcount_block_cache = qcow2_cache_create(bs, REFCOUNT_CACHE_SIZE);
> +    if (s->l2_table_cache == NULL || s->refcount_block_cache == NULL) {
> +        error_setg(errp, "Could not allocate metadata caches");
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
>  
>      s->cluster_cache = g_malloc(s->cluster_size);
>      /* one more sector for decompressed data alignment */
> -    s->cluster_data = qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size
> -                                  + 512);
> +    s->cluster_data = qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
> +                                              * s->cluster_size + 512);
> +    if (s->cluster_data == NULL) {
> +        error_setg(errp, "Could not allocate temporary cluster buffer");
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
> +
>      s->cluster_cache_offset = -1;
>      s->flags = flags;
>  
> @@ -840,7 +856,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
>      cleanup_unknown_header_ext(bs);
>      qcow2_free_snapshots(bs);
>      qcow2_refcount_close(bs);
> -    g_free(s->l1_table);
> +    qemu_vfree(s->l1_table);
>      /* else pre-write overlap checks in cache_destroy may crash */
>      s->l1_table = NULL;
>      if (s->l2_table_cache) {
> @@ -1063,7 +1079,12 @@ static coroutine_fn int qcow2_co_readv(BlockDriverState *bs, int64_t sector_num,
>                   */
>                  if (!cluster_data) {
>                      cluster_data =
> -                        qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS * s->cluster_size);
> +                        qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
> +                                                * s->cluster_size);
> +                    if (cluster_data == NULL) {
> +                        ret = -ENOMEM;
> +                        goto fail;
> +                    }
>                  }
>  
>                  assert(cur_nr_sectors <=
> @@ -1163,8 +1184,12 @@ static coroutine_fn int qcow2_co_writev(BlockDriverState *bs,
>  
>          if (s->crypt_method) {
>              if (!cluster_data) {
> -                cluster_data = qemu_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS *
> -                                                 s->cluster_size);
> +                cluster_data = qemu_try_blockalign(bs, QCOW_MAX_CRYPT_CLUSTERS
> +                                                       * s->cluster_size);
> +                if (cluster_data == NULL) {
> +                    ret = -ENOMEM;
> +                    goto fail;
> +                }
>              }
>  
>              assert(hd_qiov.size <=
> @@ -1251,7 +1276,7 @@ fail:
>  static void qcow2_close(BlockDriverState *bs)
>  {
>      BDRVQcowState *s = bs->opaque;
> -    g_free(s->l1_table);
> +    qemu_vfree(s->l1_table);
>      /* else pre-write overlap checks in cache_destroy may crash */
>      s->l1_table = NULL;
>  
> -- 
> 1.8.3.1
> 
> 

[Qemu-devel] [PATCH v3 12/21] qed: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the qed block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qed-check.c | 7 +++++--
 block/qed.c       | 6 +++++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/block/qed-check.c b/block/qed-check.c
index b473dcd..40a882c 100644
--- a/block/qed-check.c
+++ b/block/qed-check.c
@@ -227,8 +227,11 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
     };
     int ret;
 
-    check.used_clusters = g_malloc0(((check.nclusters + 31) / 32) *
-                                       sizeof(check.used_clusters[0]));
+    check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) *
+                                        sizeof(check.used_clusters[0]));
+    if (check.nclusters && check.used_clusters == NULL) {
+        return -ENOMEM;
+    }
 
     check.result->bfi.total_clusters =
         (s->header.image_size + s->header.cluster_size - 1) /
diff --git a/block/qed.c b/block/qed.c
index c130e42..f0943d6 100644
--- a/block/qed.c
+++ b/block/qed.c
@@ -1208,7 +1208,11 @@ static void qed_aio_write_inplace(QEDAIOCB *acb, uint64_t offset, size_t len)
         struct iovec *iov = acb->qiov->iov;
 
         if (!iov->iov_base) {
-            iov->iov_base = qemu_blockalign(acb->common.bs, iov->iov_len);
+            iov->iov_base = qemu_try_blockalign(acb->common.bs, iov->iov_len);
+            if (iov->iov_base == NULL) {
+                qed_aio_complete(acb, -ENOMEM);
+                return;
+            }
             memset(iov->iov_base, 0, iov->iov_len);
         }
     }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 12/21] qed: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:53 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the qed block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/qed-check.c | 7 +++++--
>  block/qed.c       | 6 +++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/block/qed-check.c b/block/qed-check.c
> index b473dcd..40a882c 100644
> --- a/block/qed-check.c
> +++ b/block/qed-check.c
> @@ -227,8 +227,11 @@ int qed_check(BDRVQEDState *s, BdrvCheckResult *result, bool fix)
>      };
>      int ret;
>  
> -    check.used_clusters = g_malloc0(((check.nclusters + 31) / 32) *
> -                                       sizeof(check.used_clusters[0]));
> +    check.used_clusters = g_try_malloc0(((check.nclusters + 31) / 32) *
> +                                        sizeof(check.used_clusters[0]));
> +    if (check.nclusters && check.used_clusters == NULL) {
> +        return -ENOMEM;
> +    }
>  
>      check.result->bfi.total_clusters =
>          (s->header.image_size + s->header.cluster_size - 1) /
> diff --git a/block/qed.c b/block/qed.c
> index c130e42..f0943d6 100644
> --- a/block/qed.c
> +++ b/block/qed.c
> @@ -1208,7 +1208,11 @@ static void qed_aio_write_inplace(QEDAIOCB *acb, uint64_t offset, size_t len)
>          struct iovec *iov = acb->qiov->iov;
>  
>          if (!iov->iov_base) {
> -            iov->iov_base = qemu_blockalign(acb->common.bs, iov->iov_len);
> +            iov->iov_base = qemu_try_blockalign(acb->common.bs, iov->iov_len);
> +            if (iov->iov_base == NULL) {
> +                qed_aio_complete(acb, -ENOMEM);
> +                return;
> +            }
>              memset(iov->iov_base, 0, iov->iov_len);
>          }
>      }
> -- 
> 1.8.3.1
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 13/21] raw-posix: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the raw-posix block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/raw-posix.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/raw-posix.c b/block/raw-posix.c
index b7f0f26..0cdf957 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -753,7 +753,11 @@ static ssize_t handle_aiocb_rw(RawPosixAIOData *aiocb)
      * Ok, we have to do it the hard way, copy all segments into
      * a single aligned buffer.
      */
-    buf = qemu_blockalign(aiocb->bs, aiocb->aio_nbytes);
+    buf = qemu_try_blockalign(aiocb->bs, aiocb->aio_nbytes);
+    if (buf == NULL) {
+        return -ENOMEM;
+    }
+
     if (aiocb->aio_type & QEMU_AIO_WRITE) {
         char *p = buf;
         int i;
-- 
1.8.3.1

[Qemu-devel] [PATCH v3 14/21] raw-win32: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the raw-win32 block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/win32-aio.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/win32-aio.c b/block/win32-aio.c
index 5d1d199..b8320ce 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -138,7 +138,10 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
     waiocb->is_read = (type == QEMU_AIO_READ);
 
     if (qiov->niov > 1) {
-        waiocb->buf = qemu_blockalign(bs, qiov->size);
+        waiocb->buf = qemu_try_blockalign(bs, qiov->size);
+        if (waiocb->buf == NULL) {
+            goto out;
+        }
         if (type & QEMU_AIO_WRITE) {
             iov_to_buf(qiov->iov, qiov->niov, 0, waiocb->buf, qiov->size);
         }
@@ -167,6 +170,7 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
 
 out_dec_count:
     aio->count--;
+out:
     qemu_aio_release(waiocb);
     return NULL;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 14/21] raw-win32: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:55 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the raw-win32 block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/win32-aio.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/block/win32-aio.c b/block/win32-aio.c
> index 5d1d199..b8320ce 100644
> --- a/block/win32-aio.c
> +++ b/block/win32-aio.c
> @@ -138,7 +138,10 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
>      waiocb->is_read = (type == QEMU_AIO_READ);
>  
>      if (qiov->niov > 1) {
> -        waiocb->buf = qemu_blockalign(bs, qiov->size);
> +        waiocb->buf = qemu_try_blockalign(bs, qiov->size);
> +        if (waiocb->buf == NULL) {
Would taking care that errno is still -ENOMEM at the failure exit of the fonction
usefull for the potentials callers ?

> +            goto out;
> +        }
>          if (type & QEMU_AIO_WRITE) {
>              iov_to_buf(qiov->iov, qiov->niov, 0, waiocb->buf, qiov->size);
>          }
> @@ -167,6 +170,7 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
>  
>  out_dec_count:
>      aio->count--;
> +out:
>      qemu_aio_release(waiocb);
>      return NULL;
>  }
> -- 
> 1.8.3.1
> 

Re: [Qemu-devel] [PATCH v3 14/21] raw-win32: Handle failure for potentially large allocations

[Kevin Wolf]

Am 03.06.2014 um 17:46 hat Benoît Canet geschrieben:
> The Tuesday 03 Jun 2014 à 15:10:55 (+0200), Kevin Wolf wrote :
> > Some code in the block layer makes potentially huge allocations. Failure
> > is not completely unexpected there, so avoid aborting qemu and handle
> > out-of-memory situations gracefully.
> > 
> > This patch addresses the allocations in the raw-win32 block driver.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  block/win32-aio.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/block/win32-aio.c b/block/win32-aio.c
> > index 5d1d199..b8320ce 100644
> > --- a/block/win32-aio.c
> > +++ b/block/win32-aio.c
> > @@ -138,7 +138,10 @@ BlockDriverAIOCB *win32_aio_submit(BlockDriverState *bs,
> >      waiocb->is_read = (type == QEMU_AIO_READ);
> >  
> >      if (qiov->niov > 1) {
> > -        waiocb->buf = qemu_blockalign(bs, qiov->size);
> > +        waiocb->buf = qemu_try_blockalign(bs, qiov->size);
> > +        if (waiocb->buf == NULL) {
> Would taking care that errno is still -ENOMEM at the failure exit of the fonction
> usefull for the potentials callers ?

There are only two callers and they don't care about errno.

Kevin

[Qemu-devel] [PATCH v3 15/21] rbd: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the rbd block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/rbd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 09af484..d0b2329 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -623,7 +623,7 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
                                        RBDAIOCmd cmd)
 {
     RBDAIOCB *acb;
-    RADOSCB *rcb;
+    RADOSCB *rcb = NULL;
     rbd_completion_t c;
     int64_t off, size;
     char *buf;
@@ -637,7 +637,10 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
     if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
         acb->bounce = NULL;
     } else {
-        acb->bounce = qemu_blockalign(bs, qiov->size);
+        acb->bounce = qemu_try_blockalign(bs, qiov->size);
+        if (acb->bounce == NULL) {
+            goto failed;
+        }
     }
     acb->ret = 0;
     acb->error = 0;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 15/21] rbd: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:56 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the rbd block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/rbd.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/block/rbd.c b/block/rbd.c
> index 09af484..d0b2329 100644
> --- a/block/rbd.c
> +++ b/block/rbd.c
> @@ -623,7 +623,7 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
>                                         RBDAIOCmd cmd)
>  {
>      RBDAIOCB *acb;
> -    RADOSCB *rcb;
> +    RADOSCB *rcb = NULL;
>      rbd_completion_t c;
>      int64_t off, size;
>      char *buf;
> @@ -637,7 +637,10 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
>      if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
>          acb->bounce = NULL;
>      } else {
> -        acb->bounce = qemu_blockalign(bs, qiov->size);
> +        acb->bounce = qemu_try_blockalign(bs, qiov->size);
I am under the impression that acb->bounce will be leaked in next goto failed.

> +        if (acb->bounce == NULL) {
> +            goto failed;
> +        }
>      }
>      acb->ret = 0;
>      acb->error = 0;
> -- 
> 1.8.3.1
> 
> 

Re: [Qemu-devel] [PATCH v3 15/21] rbd: Handle failure for potentially large allocations

[Kevin Wolf]

Am 03.06.2014 um 17:43 hat Benoît Canet geschrieben:
> The Tuesday 03 Jun 2014 à 15:10:56 (+0200), Kevin Wolf wrote :
> > Some code in the block layer makes potentially huge allocations. Failure
> > is not completely unexpected there, so avoid aborting qemu and handle
> > out-of-memory situations gracefully.
> > 
> > This patch addresses the allocations in the rbd block driver.
> > 
> > Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > ---
> >  block/rbd.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> > 
> > diff --git a/block/rbd.c b/block/rbd.c
> > index 09af484..d0b2329 100644
> > --- a/block/rbd.c
> > +++ b/block/rbd.c
> > @@ -623,7 +623,7 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
> >                                         RBDAIOCmd cmd)
> >  {
> >      RBDAIOCB *acb;
> > -    RADOSCB *rcb;
> > +    RADOSCB *rcb = NULL;
> >      rbd_completion_t c;
> >      int64_t off, size;
> >      char *buf;
> > @@ -637,7 +637,10 @@ static BlockDriverAIOCB *rbd_start_aio(BlockDriverState *bs,
> >      if (cmd == RBD_AIO_DISCARD || cmd == RBD_AIO_FLUSH) {
> >          acb->bounce = NULL;
> >      } else {
> > -        acb->bounce = qemu_blockalign(bs, qiov->size);
> > +        acb->bounce = qemu_try_blockalign(bs, qiov->size);
> I am under the impression that acb->bounce will be leaked in next goto failed.

Yes, I think you're right. That's a preexisting problem, though. I'll
fix it in a patch independent from this series.

Kevin

[Qemu-devel] [PATCH v3 16/21] vdi: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the vdi block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vdi.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index 27737af..1f76441 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -293,7 +293,12 @@ static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res,
         return -ENOTSUP;
     }
 
-    bmap = g_malloc(s->header.blocks_in_image * sizeof(uint32_t));
+    bmap = g_try_malloc(s->header.blocks_in_image * sizeof(uint32_t));
+    if (s->header.blocks_in_image && bmap == NULL) {
+        res->check_errors++;
+        return -ENOMEM;
+    }
+
     memset(bmap, 0xff, s->header.blocks_in_image * sizeof(uint32_t));
 
     /* Check block map and value of blocks_allocated. */
@@ -472,7 +477,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
 
     bmap_size = header.blocks_in_image * sizeof(uint32_t);
     bmap_size = (bmap_size + SECTOR_SIZE - 1) / SECTOR_SIZE;
-    s->bmap = g_malloc(bmap_size * SECTOR_SIZE);
+    s->bmap = qemu_try_blockalign(bs->file, bmap_size * SECTOR_SIZE);
+    if (s->bmap == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
+
     ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size);
     if (ret < 0) {
         goto fail_free_bmap;
@@ -487,7 +497,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     return 0;
 
  fail_free_bmap:
-    g_free(s->bmap);
+    qemu_vfree(s->bmap);
 
  fail:
     return ret;
@@ -760,7 +770,11 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
     }
 
     if (bmap_size > 0) {
-        uint32_t *bmap = g_malloc0(bmap_size);
+        uint32_t *bmap = g_try_malloc0(bmap_size);
+        if (bmap == NULL) {
+            result = -ENOMEM;
+            goto close_and_exit;
+        }
         for (i = 0; i < blocks; i++) {
             if (image_type == VDI_TYPE_STATIC) {
                 bmap[i] = i;
@@ -796,7 +810,7 @@ static void vdi_close(BlockDriverState *bs)
 {
     BDRVVdiState *s = bs->opaque;
 
-    g_free(s->bmap);
+    qemu_vfree(s->bmap);
 
     migrate_del_blocker(s->migration_blocker);
     error_free(s->migration_blocker);
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 16/21] vdi: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:57 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the vdi block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vdi.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index 27737af..1f76441 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -293,7 +293,12 @@ static int vdi_check(BlockDriverState *bs, BdrvCheckResult *res,
>          return -ENOTSUP;
>      }
>  
> -    bmap = g_malloc(s->header.blocks_in_image * sizeof(uint32_t));
> +    bmap = g_try_malloc(s->header.blocks_in_image * sizeof(uint32_t));
> +    if (s->header.blocks_in_image && bmap == NULL) {
> +        res->check_errors++;
> +        return -ENOMEM;
> +    }
> +
>      memset(bmap, 0xff, s->header.blocks_in_image * sizeof(uint32_t));
>  
>      /* Check block map and value of blocks_allocated. */
> @@ -472,7 +477,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>  
>      bmap_size = header.blocks_in_image * sizeof(uint32_t);
>      bmap_size = (bmap_size + SECTOR_SIZE - 1) / SECTOR_SIZE;
> -    s->bmap = g_malloc(bmap_size * SECTOR_SIZE);
> +    s->bmap = qemu_try_blockalign(bs->file, bmap_size * SECTOR_SIZE);
> +    if (s->bmap == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
> +
>      ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size);
>      if (ret < 0) {
>          goto fail_free_bmap;
> @@ -487,7 +497,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
>      return 0;
>  
>   fail_free_bmap:
> -    g_free(s->bmap);
> +    qemu_vfree(s->bmap);
>  
>   fail:
>      return ret;
> @@ -760,7 +770,11 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
>      }
>  
>      if (bmap_size > 0) {
> -        uint32_t *bmap = g_malloc0(bmap_size);
> +        uint32_t *bmap = g_try_malloc0(bmap_size);
> +        if (bmap == NULL) {
> +            result = -ENOMEM;
> +            goto close_and_exit;
> +        }
>          for (i = 0; i < blocks; i++) {
>              if (image_type == VDI_TYPE_STATIC) {
>                  bmap[i] = i;
> @@ -796,7 +810,7 @@ static void vdi_close(BlockDriverState *bs)
>  {
>      BDRVVdiState *s = bs->opaque;
>  
> -    g_free(s->bmap);
> +    qemu_vfree(s->bmap);
>  
>      migrate_del_blocker(s->migration_blocker);
>      error_free(s->migration_blocker);
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 17/21] vhdx: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the vhdx block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vhdx-log.c |  6 +++++-
 block/vhdx.c     | 12 ++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/block/vhdx-log.c b/block/vhdx-log.c
index a77c040..3eb7e68 100644
--- a/block/vhdx-log.c
+++ b/block/vhdx-log.c
@@ -349,7 +349,11 @@ static int vhdx_log_read_desc(BlockDriverState *bs, BDRVVHDXState *s,
     }
 
     desc_sectors = vhdx_compute_desc_sectors(hdr.descriptor_count);
-    desc_entries = qemu_blockalign(bs, desc_sectors * VHDX_LOG_SECTOR_SIZE);
+    desc_entries = qemu_try_blockalign(bs, desc_sectors * VHDX_LOG_SECTOR_SIZE);
+    if (desc_entries == NULL) {
+        ret = -ENOMEM;
+        goto exit;
+    }
 
     ret = vhdx_log_read_sectors(bs, log, &sectors_read, desc_entries,
                                 desc_sectors, false);
diff --git a/block/vhdx.c b/block/vhdx.c
index 353c74d..0922f55 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -950,7 +950,11 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* s->bat is freed in vhdx_close() */
-    s->bat = qemu_blockalign(bs, s->bat_rt.length);
+    s->bat = qemu_try_blockalign(bs, s->bat_rt.length);
+    if (s->bat == NULL) {
+        ret = -ENOMEM;
+        goto fail;
+    }
 
     ret = bdrv_pread(bs->file, s->bat_offset, s->bat, s->bat_rt.length);
     if (ret < 0) {
@@ -1579,7 +1583,11 @@ static int vhdx_create_bat(BlockDriverState *bs, BDRVVHDXState *s,
                 use_zero_blocks ||
                 bdrv_has_zero_init(bs) == 0) {
         /* for a fixed file, the default BAT entry is not zero */
-        s->bat = g_malloc0(rt_bat->length);
+        s->bat = g_try_malloc0(rt_bat->length);
+        if (rt_bat->length && s->bat != NULL) {
+            ret = -ENOMEM;
+            goto exit;
+        }
         block_state = type == VHDX_TYPE_FIXED ? PAYLOAD_BLOCK_FULLY_PRESENT :
                                                 PAYLOAD_BLOCK_NOT_PRESENT;
         block_state = use_zero_blocks ? PAYLOAD_BLOCK_ZERO : block_state;
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 17/21] vhdx: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:58 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the vhdx block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vhdx-log.c |  6 +++++-
>  block/vhdx.c     | 12 ++++++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
> index a77c040..3eb7e68 100644
> --- a/block/vhdx-log.c
> +++ b/block/vhdx-log.c
> @@ -349,7 +349,11 @@ static int vhdx_log_read_desc(BlockDriverState *bs, BDRVVHDXState *s,
>      }
>  
>      desc_sectors = vhdx_compute_desc_sectors(hdr.descriptor_count);
> -    desc_entries = qemu_blockalign(bs, desc_sectors * VHDX_LOG_SECTOR_SIZE);
> +    desc_entries = qemu_try_blockalign(bs, desc_sectors * VHDX_LOG_SECTOR_SIZE);
> +    if (desc_entries == NULL) {
> +        ret = -ENOMEM;
> +        goto exit;
> +    }
>  
>      ret = vhdx_log_read_sectors(bs, log, &sectors_read, desc_entries,
>                                  desc_sectors, false);
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 353c74d..0922f55 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -950,7 +950,11 @@ static int vhdx_open(BlockDriverState *bs, QDict *options, int flags,
>      }
>  
>      /* s->bat is freed in vhdx_close() */
> -    s->bat = qemu_blockalign(bs, s->bat_rt.length);
> +    s->bat = qemu_try_blockalign(bs, s->bat_rt.length);
> +    if (s->bat == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
>  
>      ret = bdrv_pread(bs->file, s->bat_offset, s->bat, s->bat_rt.length);
>      if (ret < 0) {
> @@ -1579,7 +1583,11 @@ static int vhdx_create_bat(BlockDriverState *bs, BDRVVHDXState *s,
>                  use_zero_blocks ||
>                  bdrv_has_zero_init(bs) == 0) {
>          /* for a fixed file, the default BAT entry is not zero */
> -        s->bat = g_malloc0(rt_bat->length);
> +        s->bat = g_try_malloc0(rt_bat->length);
> +        if (rt_bat->length && s->bat != NULL) {
> +            ret = -ENOMEM;
> +            goto exit;
> +        }
>          block_state = type == VHDX_TYPE_FIXED ? PAYLOAD_BLOCK_FULLY_PRESENT :
>                                                  PAYLOAD_BLOCK_NOT_PRESENT;
>          block_state = use_zero_blocks ? PAYLOAD_BLOCK_ZERO : block_state;
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 18/21] vmdk: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the vmdk block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vmdk.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 2b38f61..fd81b1f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -448,7 +448,11 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
 
     /* read the L1 table */
     l1_size = extent->l1_size * sizeof(uint32_t);
-    extent->l1_table = g_malloc(l1_size);
+    extent->l1_table = g_try_malloc(l1_size);
+    if (l1_size && extent->l1_table == NULL) {
+        return -ENOMEM;
+    }
+
     ret = bdrv_pread(extent->file,
                      extent->l1_table_offset,
                      extent->l1_table,
@@ -464,7 +468,11 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
     }
 
     if (extent->l1_backup_table_offset) {
-        extent->l1_backup_table = g_malloc(l1_size);
+        extent->l1_backup_table = g_try_malloc(l1_size);
+        if (l1_size && extent->l1_backup_table == NULL) {
+            ret = -ENOMEM;
+            goto fail_l1;
+        }
         ret = bdrv_pread(extent->file,
                          extent->l1_backup_table_offset,
                          extent->l1_backup_table,
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 18/21] vmdk: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:59 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the vmdk block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vmdk.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 2b38f61..fd81b1f 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -448,7 +448,11 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
>  
>      /* read the L1 table */
>      l1_size = extent->l1_size * sizeof(uint32_t);
> -    extent->l1_table = g_malloc(l1_size);
> +    extent->l1_table = g_try_malloc(l1_size);
> +    if (l1_size && extent->l1_table == NULL) {
> +        return -ENOMEM;
> +    }
> +
>      ret = bdrv_pread(extent->file,
>                       extent->l1_table_offset,
>                       extent->l1_table,
> @@ -464,7 +468,11 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent,
>      }
>  
>      if (extent->l1_backup_table_offset) {
> -        extent->l1_backup_table = g_malloc(l1_size);
> +        extent->l1_backup_table = g_try_malloc(l1_size);
> +        if (l1_size && extent->l1_backup_table == NULL) {
> +            ret = -ENOMEM;
> +            goto fail_l1;
> +        }
>          ret = bdrv_pread(extent->file,
>                           extent->l1_backup_table_offset,
>                           extent->l1_backup_table,
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 19/21] vpc: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the vpc block driver.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vpc.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/block/vpc.c b/block/vpc.c
index 2e25f57..a6346bb 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -269,7 +269,11 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
             goto fail;
         }
 
-        s->pagetable = qemu_blockalign(bs, s->max_table_entries * 4);
+        s->pagetable = qemu_try_blockalign(bs, s->max_table_entries * 4);
+        if (s->pagetable == NULL) {
+            ret = -ENOMEM;
+            goto fail;
+        }
 
         s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 19/21] vpc: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:11:00 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the vpc block driver.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/vpc.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/block/vpc.c b/block/vpc.c
> index 2e25f57..a6346bb 100644
> --- a/block/vpc.c
> +++ b/block/vpc.c
> @@ -269,7 +269,11 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
>              goto fail;
>          }
>  
> -        s->pagetable = qemu_blockalign(bs, s->max_table_entries * 4);
> +        s->pagetable = qemu_try_blockalign(bs, s->max_table_entries * 4);
> +        if (s->pagetable == NULL) {
> +            ret = -ENOMEM;
> +            goto fail;
> +        }
>  
>          s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
>  
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 20/21] mirror: Handle failure for potentially large allocations

[Kevin Wolf]

Some code in the block layer makes potentially huge allocations. Failure
is not completely unexpected there, so avoid aborting qemu and handle
out-of-memory situations gracefully.

This patch addresses the allocations in the mirror block job.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/mirror.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/mirror.c b/block/mirror.c
index 94c8661..07417d7 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -350,7 +350,12 @@ static void coroutine_fn mirror_run(void *opaque)
     }
 
     end = s->common.len >> BDRV_SECTOR_BITS;
-    s->buf = qemu_blockalign(bs, s->buf_size);
+    s->buf = qemu_try_blockalign(bs, s->buf_size);
+    if (s->buf == NULL) {
+        ret = -ENOMEM;
+        goto immediate_exit;
+    }
+
     sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
     mirror_free_init(s);
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 20/21] mirror: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:11:01 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the mirror block job.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> ---
>  block/mirror.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/block/mirror.c b/block/mirror.c
> index 94c8661..07417d7 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -350,7 +350,12 @@ static void coroutine_fn mirror_run(void *opaque)
>      }
>  
>      end = s->common.len >> BDRV_SECTOR_BITS;
> -    s->buf = qemu_blockalign(bs, s->buf_size);
> +    s->buf = qemu_try_blockalign(bs, s->buf_size);
> +    if (s->buf == NULL) {
> +        ret = -ENOMEM;
> +        goto immediate_exit;
> +    }
> +
>      sectors_per_chunk = s->granularity >> BDRV_SECTOR_BITS;
>      mirror_free_init(s);
>  
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

[Qemu-devel] [PATCH v3 21/21] qcow2: Return useful error code in refcount_init()

[Kevin Wolf]

From: Max Reitz <mreitz@redhat.com>

If bdrv_pread() returns an error, it is very unlikely that it was
ENOMEM. In this case, the return value should be passed along; as
bdrv_pread() will always either return the number of bytes read or a
negative value (the error code), the condition for checking whether
bdrv_pread() failed can be simplified (and clarified) as well.

Signed-off-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-refcount.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index a234c7a..6b367c8 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -49,19 +49,21 @@ int qcow2_refcount_init(BlockDriverState *bs)
 
     if (s->refcount_table_size > 0) {
         if (s->refcount_table == NULL) {
+            ret = -ENOMEM;
             goto fail;
         }
         BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_LOAD);
         ret = bdrv_pread(bs->file, s->refcount_table_offset,
                          s->refcount_table, refcount_table_size2);
-        if (ret != refcount_table_size2)
+        if (ret < 0) {
             goto fail;
+        }
         for(i = 0; i < s->refcount_table_size; i++)
             be64_to_cpus(&s->refcount_table[i]);
     }
     return 0;
  fail:
-    return -ENOMEM;
+    return ret;
 }
 
 void qcow2_refcount_close(BlockDriverState *bs)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH v3 21/21] qcow2: Return useful error code in refcount_init()

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:11:02 (+0200), Kevin Wolf wrote :
> From: Max Reitz <mreitz@redhat.com>
> 
> If bdrv_pread() returns an error, it is very unlikely that it was
> ENOMEM. In this case, the return value should be passed along; as
> bdrv_pread() will always either return the number of bytes read or a
> negative value (the error code), the condition for checking whether
> bdrv_pread() failed can be simplified (and clarified) as well.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2-refcount.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
> index a234c7a..6b367c8 100644
> --- a/block/qcow2-refcount.c
> +++ b/block/qcow2-refcount.c
> @@ -49,19 +49,21 @@ int qcow2_refcount_init(BlockDriverState *bs)
>  
>      if (s->refcount_table_size > 0) {
>          if (s->refcount_table == NULL) {
> +            ret = -ENOMEM;
>              goto fail;
>          }
>          BLKDBG_EVENT(bs->file, BLKDBG_REFTABLE_LOAD);
>          ret = bdrv_pread(bs->file, s->refcount_table_offset,
>                           s->refcount_table, refcount_table_size2);
> -        if (ret != refcount_table_size2)
> +        if (ret < 0) {
>              goto fail;
> +        }
>          for(i = 0; i < s->refcount_table_size; i++)
>              be64_to_cpus(&s->refcount_table[i]);
>      }
>      return 0;
>   fail:
> -    return -ENOMEM;
> +    return ret;
>  }
>  
>  void qcow2_refcount_close(BlockDriverState *bs)
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>