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

[Qemu-devel] [PATCH v4 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.

v4:
- Patch 11 (qcow2): Fix memory leak in qcow2_cache_create() [Benoît]

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]

*** BLURB HERE ***

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    | 13 +++++++++++-
 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, 365 insertions(+), 91 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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 v4 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 v4 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 v4 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 v4 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 v4 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 v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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 v4 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 v4 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    | 13 ++++++++++++-
 block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
 block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
 block/qcow2-snapshot.c | 22 +++++++++++++++++-----
 block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
 5 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
index 8ecbb5b..b3fe851 100644
--- a/block/qcow2-cache.c
+++ b/block/qcow2-cache.c
@@ -53,10 +53,21 @@ 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->entries);
+    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 v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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 v4 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

[Qemu-devel] [PATCH v4 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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

[Qemu-devel] [PATCH v4 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>
Reviewed-by: Benoit Canet <benoit@irqsave.net>
---
 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 v4 11/21] qcow2: Handle failure for potentially large allocations

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:23 (+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    | 13 ++++++++++++-
>  block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
>  block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
>  block/qcow2-snapshot.c | 22 +++++++++++++++++-----
>  block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
>  5 files changed, 127 insertions(+), 32 deletions(-)
> 
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index 8ecbb5b..b3fe851 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
> @@ -53,10 +53,21 @@ 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->entries);
> +    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);
I think we need asset(nb_clusters >= 1); else it will trigger a spurious -ENOMEM
on very small files.

> -    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
> 
> 

Aside from this

Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:25 (+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-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
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:22 (+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 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;
> +    }

I notice we don't sett errp on the above bdrv_pread failure.

>  
>      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:
o>      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
> 
> 

Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:27 (+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);
> +        if (acb->bounce == NULL) {
> +            goto failed;
> +        }
>      }
>      acb->ret = 0;
>      acb->error = 0;
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:26 (+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) {
> +            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--;
Isn't waiocb->buf leaked on this path ?
> +out:
>      qemu_aio_release(waiocb);
>      return NULL;
>  }
> -- 
> 1.8.3.1
> 
> 

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

[Kevin Wolf]

Am 05.06.2014 um 16:52 hat Benoît Canet geschrieben:
> The Thursday 05 Jun 2014 à 15:36:23 (+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    | 13 ++++++++++++-
> >  block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
> >  block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
> >  block/qcow2-snapshot.c | 22 +++++++++++++++++-----
> >  block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
> >  5 files changed, 127 insertions(+), 32 deletions(-)

> > @@ -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);
> I think we need asset(nb_clusters >= 1); else it will trigger a spurious -ENOMEM
> on very small files.

To be precise, very small means size 0, because size_to_clusters() rounds
up. An image file with size 0 wouldn't have a qcow2 header, though, so we
wouldn't even have opened it in the first place.

(Also, an assert() never fixes a bug. At best, it changes its symptom to
a crash, except when built with NDEBUG defined.)

> > -    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);

Kevin

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

[Kevin Wolf]

Am 05.06.2014 um 17:09 hat Benoît Canet geschrieben:
> The Thursday 05 Jun 2014 à 15:36:26 (+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) {
> > +            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--;
> Isn't waiocb->buf leaked on this path ?

The already existing error path you mean? Looks like it, feel free to
send a patch.

Kevin

> > +out:
> >      qemu_aio_release(waiocb);
> >      return NULL;
> >  }
> > -- 
> > 1.8.3.1
> > 
> > 

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:23 (+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    | 13 ++++++++++++-
>  block/qcow2-cluster.c  | 35 +++++++++++++++++++++++++++--------
>  block/qcow2-refcount.c | 48 ++++++++++++++++++++++++++++++++++++++----------
>  block/qcow2-snapshot.c | 22 +++++++++++++++++-----
>  block/qcow2.c          | 41 +++++++++++++++++++++++++++++++++--------
>  5 files changed, 127 insertions(+), 32 deletions(-)
> 
> diff --git a/block/qcow2-cache.c b/block/qcow2-cache.c
> index 8ecbb5b..b3fe851 100644
> --- a/block/qcow2-cache.c
> +++ b/block/qcow2-cache.c
> @@ -53,10 +53,21 @@ 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->entries);
> +    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
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Benoît Canet]

The Thursday 05 Jun 2014 à 15:36:26 (+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) {
> +            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
> 
> 
Reviewed-by: Benoit Canet <benoit@irqsave.net>

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

[Stefan Hajnoczi]

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

On Thu, Jun 05, 2014 at 03:36:12PM +0200, Kevin Wolf wrote:
> 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.
> 
> v4:
> - Patch 11 (qcow2): Fix memory leak in qcow2_cache_create() [Benoît]
> 
> 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]
> 
> *** BLURB HERE ***
> 
> 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    | 13 +++++++++++-
>  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, 365 insertions(+), 91 deletions(-)
> 
> -- 
> 1.8.3.1
> 

Thanks, applied to my block tree:
https://github.com/stefanha/qemu/commits/block

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

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

[Stefan Hajnoczi]

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

On Thu, Jun 05, 2014 at 03:36:12PM +0200, Kevin Wolf wrote:
> 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.
> 
> v4:
> - Patch 11 (qcow2): Fix memory leak in qcow2_cache_create() [Benoît]
> 
> 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]

Turns out the qemu_try_blockalign() assertion is being triggered by
qemu-iotests.  Please rerun ./check && ./check -qcow2.

I've dropped it from the block queue.

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]

[Qemu-devel] [PATCH v4 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