Re: [PATCH v2] virtio-gpu: use g_try_malloc to avoid guest-triggered abort

Tue, 28 Jul 2026 22:40:21 -0700 

On 2026/07/29 7:03, Marc-André Lureau wrote:

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(&region);
+                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..
I'm not sure what a reasonable limit would be. With fully fragmented 4 KiB mappings, 16384 * 16 IOVs represent only 1 GiB of backing.
The bound in [1] applies only to non-virgl blobs. This mapping code is also used for virgl resource backing. Such resources are not limited to framebuffers and may be used for accelerated rendering or compute, so there is no similarly obvious size bound.
There is already an RFC patch to remove the 16384-entry limit for GPGPU workloads [2], so we may not derive a new limit from that value.
[1] https://lore.kernel.org/qemu-devel/[email protected]/ [2] https://lore.kernel.org/qemu-devel/[email protected]/ 

Regards,
Akihiko Odaki

Reply via email to