[PATCH v2] hw/display: don't accidentally autofree existing virgl resources

Alex Bennée posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260417122703.845442-1-alex.bennee@linaro.org
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>
hw/display/virtio-gpu-virgl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH v2] hw/display: don't accidentally autofree existing virgl resources
Posted by Alex Bennée 1 month, 1 week ago
While sanity checking a create blob operation the use of the auto
freed res variable could lead to inadvertently freeing an existing
blob.

Avoid this by in-lining the virtio_gpu_virgl_find_resource() check as
the value is not needed anyway.

While at it add a comment to the end and use g_steal_pointer to make
it clearer the object lifetime exceeds the function bounds if we pass
all the checks.

Fixes: CVE-2026-6502
Fixes: 7c092f17cce (virtio-gpu: Handle resource blob commands)
Message-ID: 20260417094443.785462-1-alex.bennee@linaro.org
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Cc: qemu-stable@nongnu.org
---
 hw/display/virtio-gpu-virgl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index b7a2d160ddd..add85bd4e61 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -830,8 +830,7 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
         return;
     }
 
-    res = virtio_gpu_virgl_find_resource(g, cblob.resource_id);
-    if (res) {
+    if (virtio_gpu_virgl_find_resource(g, cblob.resource_id)) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
                       __func__, cblob.resource_id);
         cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
@@ -884,8 +883,9 @@ static void virgl_cmd_resource_create_blob(VirtIOGPU *g,
 
     res->base.dmabuf_fd = info.fd;
 
+    /* Now live, cleaned up in virtio_gpu_virgl_resource_unref */
     QTAILQ_INSERT_HEAD(&g->reslist, &res->base, next);
-    res = NULL;
+    g_steal_pointer(&res);
 }
 
 static void virgl_cmd_resource_map_blob(VirtIOGPU *g,
-- 
2.47.3


Re: [PATCH v2] hw/display: don't accidentally autofree existing virgl resources
Posted by Alex Bennée 1 month, 1 week ago
Alex Bennée <alex.bennee@linaro.org> writes:

> While sanity checking a create blob operation the use of the auto
> freed res variable could lead to inadvertently freeing an existing
> blob.
>
> Avoid this by in-lining the virtio_gpu_virgl_find_resource() check as
> the value is not needed anyway.
>
> While at it add a comment to the end and use g_steal_pointer to make
> it clearer the object lifetime exceeds the function bounds if we pass
> all the checks.

Queued to virtio-gpu/next, thanks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro