Hi Akihiko On Tue, Jul 28, 2026 at 11:17 PM Akihiko Odaki <[email protected]> wrote: > > On 2026/07/28 23:33, Marc-André Lureau wrote: > > Hi > > > > On Tue, Jul 28, 2026 at 6:28 PM Akihiko Odaki > > <[email protected]> wrote: > >> > >> On 2026/07/28 19:15, [email protected] wrote: > >>> 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") > >>> v2: > >>> - accept NULL 0-size allocations > >>> --- > >>> 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 | 5 ++++- > >>> 6 files changed, 47 insertions(+), 14 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 8607d0c0b581..9bacd38b4521 100644 > >>> --- a/hw/display/virtio-gpu.c > >>> +++ b/hw/display/virtio-gpu.c > >>> @@ -892,7 +892,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; > >>> + } > >> > >> This converts the entry allocation to g_try_malloc(), but the resulting > >> mapping arrays still grow with aborting g_renew() calls. > >> dma_memory_map() may return only part of an entry; with fragmented IOMMU > >> mappings, guest-controlled entries can expand into more IOVs than > >> nr_entries. > >> > > > > The goal is not to make QEMU OOM-safe. Even when malloc returns > > non-zero we may reach that and get killed. This patch is more a safety > > guard for easy guest triggered allocations that can crash QEMU. > I agree we are not going to make QEMU OOM-safe, but I'm not sure where > we draw the line between "easy" and "hard" guest-triggered allocations. > Even if the IOMMU mapping isn't fragmented, the data from ents is > essentially duplicated into iov right after, doubling the allocation > size anyway. I feel it's easier to avoid reasoning by just consistently > using g_try_* in this function.
Ok, maybe it doesn't make sense for bounded-allocation to use g_try then. It's true that if the IOMMU mapping is fragmented, we could end up with an allocation that could fail. I am not sure we can do much in this case but rather have a reasonable limit like VIRTIO_GPU_MAX_IOV_ENTRIES (16384 * 16). Otherwise, happy to convert to g_try_renew() for now..
