[PATCH v2] hw/display/virtio-gpu: cap submit_3d command buffer allocation

marcandre.lureau@redhat.com posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260716105023.3246364-1-marcandre.lureau@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, Stefano Garzarella <sgarzare@redhat.com>, "Marc-André Lureau" <marcandre.lureau@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>
contrib/vhost-user-gpu/vugpu.h   | 8 ++++++++
include/hw/virtio/virtio-gpu.h   | 8 ++++++++
contrib/vhost-user-gpu/virgl.c   | 7 +++++++
hw/display/virtio-gpu-rutabaga.c | 8 ++++++++
hw/display/virtio-gpu-virgl.c    | 8 ++++++++
5 files changed, 39 insertions(+)
[PATCH v2] hw/display/virtio-gpu: cap submit_3d command buffer allocation
Posted by marcandre.lureau@redhat.com 1 week, 2 days ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Both virgl_cmd_submit_3d() and rutabaga_cmd_submit_3d() pass the
guest-controlled cs.size directly to malloc() without bounds checking. A
malicious guest can set cs.size to an arbitrarily large value, causing
an OOM abort that crashes the QEMU process.

Checking cs.size against the descriptor payload length (iov_size) is not
sufficient: indirect descriptor tables can repeat entries aliasing the
same guest-physical range, inflating iov_size() to nearly 4 GiB while
referring to only a small amount of unique memory.

Instead, cap cs.size at 4 MiB. With 4 KiB pages and QEMU's
VIRTQUEUE_MAX_SIZE (1024) mapped-iov limit, the Linux virtio driver
cannot carry more than ~4 MiB of inline command data, so legitimate
submissions are unaffected.

Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
Fixes: d52c454aadc ("contrib: add vhost-user-gpu")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3776
Reported-by: admin@fluentlogic.org
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 contrib/vhost-user-gpu/vugpu.h   | 8 ++++++++
 include/hw/virtio/virtio-gpu.h   | 8 ++++++++
 contrib/vhost-user-gpu/virgl.c   | 7 +++++++
 hw/display/virtio-gpu-rutabaga.c | 8 ++++++++
 hw/display/virtio-gpu-virgl.c    | 8 ++++++++
 5 files changed, 39 insertions(+)

diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h
index 654c392fbbf..0421c536403 100644
--- a/contrib/vhost-user-gpu/vugpu.h
+++ b/contrib/vhost-user-gpu/vugpu.h
@@ -163,6 +163,14 @@ struct virtio_gpu_ctrl_command {
     QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
 };
 
+/*
+ * With 4 KiB pages and QEMU's VIRTQUEUE_MAX_SIZE (1024) mapped-iov
+ * limit, the largest inline command is ~4 MiB.  Cap submit_3d
+ * allocations to this value to prevent a malicious guest from
+ * triggering an OOM abort via an inflated cs.size field.
+ */
+#define VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE (4 * 1024 * 1024)
+
 #define VUGPU_FILL_CMD(out) do {                                \
         size_t vugpufillcmd_s_ =                                \
             iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0,  \
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index f69fc194627..c10bfbd596f 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -299,6 +299,14 @@ struct VirtIOGPURutabaga {
     struct rutabaga *rutabaga;
 };
 
+/*
+ * With 4 KiB pages and QEMU's VIRTQUEUE_MAX_SIZE (1024) mapped-iov
+ * limit, the largest inline command is ~4 MiB.  Cap submit_3d
+ * allocations to this value to prevent a malicious guest from
+ * triggering an OOM abort via an inflated cs.size field.
+ */
+#define VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE (4 * 1024 * 1024)
+
 #define VIRTIO_GPU_FILL_CMD(out) do {                                   \
         size_t virtiogpufillcmd_s_ =                                    \
             iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0,          \
diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
index 51da0e3667f..550fd03bf5c 100644
--- a/contrib/vhost-user-gpu/virgl.c
+++ b/contrib/vhost-user-gpu/virgl.c
@@ -202,6 +202,13 @@ virgl_cmd_submit_3d(VuGpu *g,
 
     VUGPU_FILL_CMD(cs);
 
+    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
+        g_critical("%s: command buffer too large (%u)",
+                   __func__, cs.size);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        return;
+    }
+
     buf = g_malloc(cs.size);
     s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
                    sizeof(cs), buf, cs.size);
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 6ff12639012..4d7d7b24592 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -351,6 +351,14 @@ rutabaga_cmd_submit_3d(VirtIOGPU *g,
     VIRTIO_GPU_FILL_CMD(cs);
     trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
 
+    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: command buffer too large (%u)\n",
+                      __func__, cs.size);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        return;
+    }
+
     buf = g_new0(uint8_t, cs.size);
     s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
                    sizeof(cs), buf, cs.size);
diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 60c78af06a4..d9e5b011049 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -607,6 +607,14 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
     VIRTIO_GPU_FILL_CMD(cs);
     trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
 
+    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: command buffer too large (%u)\n",
+                      __func__, cs.size);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
+        return;
+    }
+
     buf = g_malloc(cs.size);
     s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
                    sizeof(cs), buf, cs.size);
-- 
2.55.0


Re: [PATCH v2] hw/display/virtio-gpu: cap submit_3d command buffer allocation
Posted by Akihiko Odaki 1 week, 2 days ago
On 2026/07/16 19:50, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Both virgl_cmd_submit_3d() and rutabaga_cmd_submit_3d() pass the
> guest-controlled cs.size directly to malloc() without bounds checking. A
> malicious guest can set cs.size to an arbitrarily large value, causing
> an OOM abort that crashes the QEMU process.
> 
> Checking cs.size against the descriptor payload length (iov_size) is not
> sufficient: indirect descriptor tables can repeat entries aliasing the
> same guest-physical range, inflating iov_size() to nearly 4 GiB while
> referring to only a small amount of unique memory.
> 
> Instead, cap cs.size at 4 MiB. With 4 KiB pages and QEMU's
> VIRTQUEUE_MAX_SIZE (1024) mapped-iov limit, the Linux virtio driver
> cannot carry more than ~4 MiB of inline command data, so legitimate
> submissions are unaffected.
> 
> Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
> Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
> Fixes: d52c454aadc ("contrib: add vhost-user-gpu")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3776
> Reported-by: admin@fluentlogic.org
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   contrib/vhost-user-gpu/vugpu.h   | 8 ++++++++
>   include/hw/virtio/virtio-gpu.h   | 8 ++++++++
>   contrib/vhost-user-gpu/virgl.c   | 7 +++++++
>   hw/display/virtio-gpu-rutabaga.c | 8 ++++++++
>   hw/display/virtio-gpu-virgl.c    | 8 ++++++++
>   5 files changed, 39 insertions(+)
> 
> diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vugpu.h
> index 654c392fbbf..0421c536403 100644
> --- a/contrib/vhost-user-gpu/vugpu.h
> +++ b/contrib/vhost-user-gpu/vugpu.h
> @@ -163,6 +163,14 @@ struct virtio_gpu_ctrl_command {
>       QTAILQ_ENTRY(virtio_gpu_ctrl_command) next;
>   };
>   
> +/*
> + * With 4 KiB pages and QEMU's VIRTQUEUE_MAX_SIZE (1024) mapped-iov
> + * limit, the largest inline command is ~4 MiB.  Cap submit_3d
> + * allocations to this value to prevent a malicious guest from
> + * triggering an OOM abort via an inflated cs.size field.
> + */
> +#define VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE (4 * 1024 * 1024)

One nit I failed to point out earlier: please use the "MiB" macro.
With that addressed:

Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Regards,
Akihiko Odaki

> +
>   #define VUGPU_FILL_CMD(out) do {                                \
>           size_t vugpufillcmd_s_ =                                \
>               iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0,  \
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index f69fc194627..c10bfbd596f 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -299,6 +299,14 @@ struct VirtIOGPURutabaga {
>       struct rutabaga *rutabaga;
>   };
>   
> +/*
> + * With 4 KiB pages and QEMU's VIRTQUEUE_MAX_SIZE (1024) mapped-iov
> + * limit, the largest inline command is ~4 MiB.  Cap submit_3d
> + * allocations to this value to prevent a malicious guest from
> + * triggering an OOM abort via an inflated cs.size field.
> + */
> +#define VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE (4 * 1024 * 1024)
> +
>   #define VIRTIO_GPU_FILL_CMD(out) do {                                   \
>           size_t virtiogpufillcmd_s_ =                                    \
>               iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num, 0,          \
> diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
> index 51da0e3667f..550fd03bf5c 100644
> --- a/contrib/vhost-user-gpu/virgl.c
> +++ b/contrib/vhost-user-gpu/virgl.c
> @@ -202,6 +202,13 @@ virgl_cmd_submit_3d(VuGpu *g,
>   
>       VUGPU_FILL_CMD(cs);
>   
> +    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
> +        g_critical("%s: command buffer too large (%u)",
> +                   __func__, cs.size);
> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +        return;
> +    }
> +
>       buf = g_malloc(cs.size);
>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>                      sizeof(cs), buf, cs.size);
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index 6ff12639012..4d7d7b24592 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -351,6 +351,14 @@ rutabaga_cmd_submit_3d(VirtIOGPU *g,
>       VIRTIO_GPU_FILL_CMD(cs);
>       trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
>   
> +    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: command buffer too large (%u)\n",
> +                      __func__, cs.size);
> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +        return;
> +    }
> +
>       buf = g_new0(uint8_t, cs.size);
>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>                      sizeof(cs), buf, cs.size);
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 60c78af06a4..d9e5b011049 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -607,6 +607,14 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
>       VIRTIO_GPU_FILL_CMD(cs);
>       trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
>   
> +    if (cs.size > VIRTIO_GPU_MAX_CMD_SUBMIT_SIZE) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: command buffer too large (%u)\n",
> +                      __func__, cs.size);
> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +        return;
> +    }
> +
>       buf = g_malloc(cs.size);
>       s = iov_to_buf(cmd->elem.out_sg, cmd->elem.out_num,
>                      sizeof(cs), buf, cs.size);