* [Qemu-devel] [PATCH v4 01/21] block: Introduce qemu_try_blockalign()
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 02/21] block: Handle failure for potentially large allocations Kevin Wolf
` (21 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 02/21] block: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 01/21] block: Introduce qemu_try_blockalign() Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 03/21] bochs: " Kevin Wolf
` (20 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 03/21] bochs: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 01/21] block: Introduce qemu_try_blockalign() Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 02/21] block: Handle failure for potentially large allocations Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 04/21] cloop: " Kevin Wolf
` (19 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 04/21] cloop: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (2 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 03/21] bochs: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 05/21] curl: " Kevin Wolf
` (18 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 05/21] curl: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (3 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 04/21] cloop: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 06/21] dmg: " Kevin Wolf
` (17 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 06/21] dmg: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (4 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 05/21] curl: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 07/21] iscsi: " Kevin Wolf
` (16 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 07/21] iscsi: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (5 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 06/21] dmg: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 08/21] nfs: " Kevin Wolf
` (15 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 08/21] nfs: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (6 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 07/21] iscsi: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 09/21] parallels: " Kevin Wolf
` (14 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 09/21] parallels: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (7 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 08/21] nfs: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 10/21] qcow1: " Kevin Wolf
` (13 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 10/21] qcow1: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (8 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 09/21] parallels: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 15:04 ` Benoît Canet
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 11/21] qcow2: " Kevin Wolf
` (12 subsequent siblings)
22 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 10/21] qcow1: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 10/21] qcow1: " Kevin Wolf
@ 2014-06-05 15:04 ` Benoît Canet
0 siblings, 0 replies; 34+ messages in thread
From: Benoît Canet @ 2014-06-05 15:04 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v4 11/21] qcow2: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (9 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 10/21] qcow1: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 14:52 ` Benoît Canet
2014-06-06 11:19 ` Benoît Canet
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 12/21] qed: " Kevin Wolf
` (11 subsequent siblings)
22 siblings, 2 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 11/21] qcow2: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 11/21] qcow2: " Kevin Wolf
@ 2014-06-05 14:52 ` Benoît Canet
2014-06-06 8:55 ` Kevin Wolf
2014-06-06 11:19 ` Benoît Canet
1 sibling, 1 reply; 34+ messages in thread
From: Benoît Canet @ 2014-06-05 14:52 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 11/21] qcow2: Handle failure for potentially large allocations
2014-06-05 14:52 ` Benoît Canet
@ 2014-06-06 8:55 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-06 8:55 UTC (permalink / raw)
To: Benoît Canet; +Cc: qemu-devel, stefanha
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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v4 11/21] qcow2: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 11/21] qcow2: " Kevin Wolf
2014-06-05 14:52 ` Benoît Canet
@ 2014-06-06 11:19 ` Benoît Canet
1 sibling, 0 replies; 34+ messages in thread
From: Benoît Canet @ 2014-06-06 11:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v4 12/21] qed: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (10 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 11/21] qcow2: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 13/21] raw-posix: " Kevin Wolf
` (10 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 13/21] raw-posix: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (11 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 12/21] qed: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 14:57 ` Benoît Canet
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 14/21] raw-win32: " Kevin Wolf
` (9 subsequent siblings)
22 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 13/21] raw-posix: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 13/21] raw-posix: " Kevin Wolf
@ 2014-06-05 14:57 ` Benoît Canet
0 siblings, 0 replies; 34+ messages in thread
From: Benoît Canet @ 2014-06-05 14:57 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v4 14/21] raw-win32: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (12 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 13/21] raw-posix: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 15:09 ` Benoît Canet
2014-06-06 11:19 ` Benoît Canet
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 15/21] rbd: " Kevin Wolf
` (8 subsequent siblings)
22 siblings, 2 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 14/21] raw-win32: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 14/21] raw-win32: " Kevin Wolf
@ 2014-06-05 15:09 ` Benoît Canet
2014-06-06 9:53 ` Kevin Wolf
2014-06-06 11:19 ` Benoît Canet
1 sibling, 1 reply; 34+ messages in thread
From: Benoît Canet @ 2014-06-05 15:09 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 14/21] raw-win32: Handle failure for potentially large allocations
2014-06-05 15:09 ` Benoît Canet
@ 2014-06-06 9:53 ` Kevin Wolf
0 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-06 9:53 UTC (permalink / raw)
To: Benoît Canet; +Cc: qemu-devel, stefanha
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
> >
> >
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [Qemu-devel] [PATCH v4 14/21] raw-win32: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 14/21] raw-win32: " Kevin Wolf
2014-06-05 15:09 ` Benoît Canet
@ 2014-06-06 11:19 ` Benoît Canet
1 sibling, 0 replies; 34+ messages in thread
From: Benoît Canet @ 2014-06-06 11:19 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v4 15/21] rbd: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (13 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 14/21] raw-win32: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 15:05 ` Benoît Canet
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 16/21] vdi: " Kevin Wolf
` (7 subsequent siblings)
22 siblings, 1 reply; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 15/21] rbd: Handle failure for potentially large allocations
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 15/21] rbd: " Kevin Wolf
@ 2014-06-05 15:05 ` Benoît Canet
0 siblings, 0 replies; 34+ messages in thread
From: Benoît Canet @ 2014-06-05 15:05 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
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>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [Qemu-devel] [PATCH v4 16/21] vdi: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (14 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 15/21] rbd: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 17/21] vhdx: " Kevin Wolf
` (6 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 17/21] vhdx: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (15 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 16/21] vdi: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 18/21] vmdk: " Kevin Wolf
` (5 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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, §ors_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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 18/21] vmdk: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (16 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 17/21] vhdx: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 19/21] vpc: " Kevin Wolf
` (4 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 19/21] vpc: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (17 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 18/21] vmdk: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 20/21] mirror: " Kevin Wolf
` (3 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 20/21] mirror: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (18 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 19/21] vpc: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 21/21] qcow2: Return useful error code in refcount_init() Kevin Wolf
` (2 subsequent siblings)
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* [Qemu-devel] [PATCH v4 21/21] qcow2: Return useful error code in refcount_init()
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (19 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 20/21] mirror: " Kevin Wolf
@ 2014-06-05 13:36 ` Kevin Wolf
2014-06-11 10:33 ` [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Stefan Hajnoczi
2014-06-11 14:36 ` Stefan Hajnoczi
22 siblings, 0 replies; 34+ messages in thread
From: Kevin Wolf @ 2014-06-05 13:36 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, benoit.canet, stefanha
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
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (20 preceding siblings ...)
2014-06-05 13:36 ` [Qemu-devel] [PATCH v4 21/21] qcow2: Return useful error code in refcount_init() Kevin Wolf
@ 2014-06-11 10:33 ` Stefan Hajnoczi
2014-06-11 14:36 ` Stefan Hajnoczi
22 siblings, 0 replies; 34+ messages in thread
From: Stefan Hajnoczi @ 2014-06-11 10:33 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel
[-- 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 --]
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations
2014-06-05 13:36 [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Kevin Wolf
` (21 preceding siblings ...)
2014-06-11 10:33 ` [Qemu-devel] [PATCH v4 00/21] block: Handle failure for potentially large allocations Stefan Hajnoczi
@ 2014-06-11 14:36 ` Stefan Hajnoczi
22 siblings, 0 replies; 34+ messages in thread
From: Stefan Hajnoczi @ 2014-06-11 14:36 UTC (permalink / raw)
To: Kevin Wolf; +Cc: benoit.canet, qemu-devel, stefanha
[-- 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 --]
^ permalink raw reply [flat|nested] 34+ messages in thread