From: Marc-André Lureau <[email protected]>
Use g_try_malloc/g_try_new0 for guest-controlled allocation, so failure
returns an error to the guest rather than crashing the host (glib
behaviour).
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3898
Signed-off-by: Marc-André Lureau <[email protected]>
---
Based-on: <[email protected]> ("[PATCH]
vhost-user-gpu: fix integer overflow in buffer allocation")
v3:
- also try allocations for SG DMA mappings
---
contrib/vhost-user-gpu/vhost-user-gpu.c | 25 +++++++++++------
contrib/vhost-user-gpu/virgl.c | 6 ++++-
contrib/vhost-user-gpu/vugbm.c | 5 +++-
hw/display/virtio-gpu-rutabaga.c | 14 ++++++++--
hw/display/virtio-gpu-virgl.c | 6 ++++-
hw/display/virtio-gpu.c | 36 +++++++++++++++++--------
6 files changed, 68 insertions(+), 24 deletions(-)
diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c
b/contrib/vhost-user-gpu/vhost-user-gpu.c
index ee9858c397ce..0fd115300425 100644
--- a/contrib/vhost-user-gpu/vhost-user-gpu.c
+++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
@@ -487,7 +487,7 @@ vg_create_mapping_iov(VuGpu *g,
struct virtio_gpu_ctrl_command *cmd,
struct iovec **iov)
{
- struct virtio_gpu_mem_entry *ents;
+ g_autofree struct virtio_gpu_mem_entry *ents = NULL;
size_t esize, s;
int i;
@@ -498,17 +498,22 @@ vg_create_mapping_iov(VuGpu *g,
}
esize = sizeof(*ents) * ab->nr_entries;
- ents = g_malloc(esize);
+ ents = g_try_malloc(esize);
+ if (!ents && esize) {
+ return -1;
+ }
s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
sizeof(*ab), ents, esize);
if (s != esize) {
g_critical("%s: command data size incorrect %zu vs %zu",
__func__, s, esize);
- g_free(ents);
return -1;
}
- *iov = g_new0(struct iovec, ab->nr_entries);
+ *iov = g_try_new0(struct iovec, ab->nr_entries);
+ if (!*iov && ab->nr_entries) {
+ return -1;
+ }
for (i = 0; i < ab->nr_entries; i++) {
uint64_t len = ents[i].length;
(*iov)[i].iov_len = ents[i].length;
@@ -517,12 +522,10 @@ vg_create_mapping_iov(VuGpu *g,
g_critical("%s: resource %d element %d",
__func__, ab->resource_id, i);
g_free(*iov);
- g_free(ents);
*iov = NULL;
return -1;
}
}
- g_free(ents);
return 0;
}
@@ -828,8 +831,14 @@ vg_resource_flush(VuGpu *g,
PIXMAN_FORMAT_BPP(pixman_image_get_format(res->image)) / 8;
size_t size = width * height * bpp;
- void *p = g_malloc(VHOST_USER_GPU_HDR_SIZE +
- sizeof(VhostUserGpuUpdate) + size);
+ void *p = g_try_malloc(VHOST_USER_GPU_HDR_SIZE +
+ sizeof(VhostUserGpuUpdate) + size);
+ if (!p) {
+ pixman_region_fini(®ion);
+ pixman_region_fini(&finalregion);
+ cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+ break;
+ }
VhostUserGpuMsg *msg = p;
msg->request = VHOST_USER_GPU_UPDATE;
msg->size = sizeof(VhostUserGpuUpdate) + size;
diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
index 550fd03bf5c4..20bae57d0fe4 100644
--- a/contrib/vhost-user-gpu/virgl.c
+++ b/contrib/vhost-user-gpu/virgl.c
@@ -209,7 +209,11 @@ virgl_cmd_submit_3d(VuGpu *g,
return;
}
- buf = g_malloc(cs.size);
+ buf = g_try_malloc(cs.size);
+ if (!buf && cs.size) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+ return;
+ }
s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
sizeof(cs), buf, cs.size);
if (s != cs.size) {
diff --git a/contrib/vhost-user-gpu/vugbm.c b/contrib/vhost-user-gpu/vugbm.c
index 710d54529779..e2d8385fd857 100644
--- a/contrib/vhost-user-gpu/vugbm.c
+++ b/contrib/vhost-user-gpu/vugbm.c
@@ -13,7 +13,10 @@
static bool
mem_alloc_bo(struct vugbm_buffer *buf)
{
- buf->mmap = g_malloc((uint64_t)buf->width * buf->height * 4);
+ buf->mmap = g_try_malloc((uint64_t)buf->width * buf->height * 4);
+ if (!buf->mmap && buf->width && buf->height) {
+ return false;
+ }
buf->stride = buf->width * 4;
return true;
}
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index e28aad94eead..60f64d9a8fa7 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -360,10 +360,20 @@ rutabaga_cmd_submit_3d(VirtIOGPU *g,
return;
}
- buf = g_new0(uint8_t, cs.size);
+ buf = g_try_new0(uint8_t, cs.size);
+ if (!buf && cs.size) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+ return;
+ }
s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
sizeof(cs), buf, cs.size);
- CHECK(s == cs.size, cmd);
+ if (s != cs.size) {
+ qemu_log_mask(LOG_GUEST_ERROR,
+ "%s: size mismatch (%zu/%u)\n",
+ __func__, s, cs.size);
+ cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+ return;
+ }
rutabaga_cmd.ctx_id = cs.hdr.ctx_id;
rutabaga_cmd.cmd = buf;
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index d9e5b0110497..028ab776df6a 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -615,7 +615,11 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
return;
}
- buf = g_malloc(cs.size);
+ buf = g_try_malloc(cs.size);
+ if (!buf && cs.size) {
+ cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+ return;
+ }
s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
sizeof(cs), buf, cs.size);
if (s != cs.size) {
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index c707f1f516ea..6894d500fb36 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -882,7 +882,10 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
}
esize = sizeof(*ents) * nr_entries;
- ents = g_malloc(esize);
+ ents = g_try_malloc(esize);
+ if (!ents && esize) {
+ return -1;
+ }
s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
offset, ents, esize);
if (s != esize) {
@@ -903,6 +906,7 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
hwaddr len;
void *map;
+ /* TODO: a common DMA map SG helper */
do {
len = l;
map = dma_memory_map(VIRTIO_DEVICE(g)->dma_as, a, &len,
@@ -911,20 +915,23 @@ int virtio_gpu_create_mapping_iov(VirtIOGPU *g,
if (!map) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: failed to map MMIO memory
for"
" element %d\n", __func__, e);
- virtio_gpu_cleanup_mapping_iov(g, *iov, v);
- g_free(ents);
- *iov = NULL;
- if (addr) {
- g_free(*addr);
- *addr = NULL;
- }
- return -1;
+ goto err;
}
if (!(v % 16)) {
- *iov = g_renew(struct iovec, *iov, v + 16);
+ struct iovec *new_iov;
+ new_iov = g_try_renew(struct iovec, *iov, v + 16);
+ if (!new_iov) {
+ goto err;
+ }
+ *iov = new_iov;
if (addr) {
- *addr = g_renew(uint64_t, *addr, v + 16);
+ uint64_t *new_addr;
+ new_addr = g_try_renew(uint64_t, *addr, v + 16);
+ if (!new_addr) {
+ goto err;
+ }
