1. Patchew
  2. QEMU
  3. View series

[PATCH] hw/display/virtio-gpu: validate command buffer size in submit_3d

marcandre.lureau@redhat.com posted 1 patch 1 week, 5 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260713125410.105653-1-marcandre.lureau@redhat.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>
hw/display/virtio-gpu-virgl.c | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH] hw/display/virtio-gpu: validate command buffer size in submit_3d
Posted by marcandre.lureau@redhat.com 1 week, 5 days ago 
From: Marc-André Lureau <marcandre.lureau@redhat.com>

virgl_cmd_submit_3d() passes the guest-controlled cs.size directly to
g_malloc() without any bounds check. A malicious guest can set this
field to an arbitrarily large value (up to 4GB), causing an OOM abort
that crashes the QEMU process.

Validate cs.size against the actual descriptor payload size before
allocating, rejecting values that exceed what the virtqueue entry
can carry.

Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
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>
---
 hw/display/virtio-gpu-virgl.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
index 60c78af06a4..2a07dd2431f 100644
--- a/hw/display/virtio-gpu-virgl.c
+++ b/hw/display/virtio-gpu-virgl.c
@@ -601,12 +601,23 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
                                 struct virtio_gpu_ctrl_command *cmd)
 {
     struct virtio_gpu_cmd_submit cs;
+    size_t iov_len;
     void *buf;
     size_t s;
 
     VIRTIO_GPU_FILL_CMD(cs);
     trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
 
+    iov_len = iov_size(cmd->elem.out_sg, cmd->elem.out_num);
+    if (cs.size == 0 || iov_len < sizeof(cs) ||
+        cs.size > iov_len - sizeof(cs)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: size out of range (%u/%zu)",
+                      __func__, cs.size, iov_len);
+        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] hw/display/virtio-gpu: validate command buffer size in submit_3d
Posted by Akihiko Odaki 1 week, 2 days ago 
On 2026/07/13 21:54, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> virgl_cmd_submit_3d() passes the guest-controlled cs.size directly to
> g_malloc() without any bounds check. A malicious guest can set this
> field to an arbitrarily large value (up to 4GB), causing an OOM abort
> that crashes the QEMU process.

virtio-gpu-rutabaga has the same problem: it also makes a fatal
guest-sized allocation before copying the payload.

> 
> Validate cs.size against the actual descriptor payload size before
> allocating, rejecting values that exceed what the virtqueue entry
> can carry.

Unfortunately, this check does not bound the allocation. The descriptor
payload length is also controlled by the guest, and descriptors can
alias the same guest-physical range. For example, an indirect descriptor
table can repeat the following entry: addr = A, len = 4 MiB

With 1,023 such entries and one response descriptor, iov_size() reports
4092 MiB even though the entries refer to only 4 MiB of unique guest
memory. A cs.size of nearly 4 GiB still passes the proposed
check and reaches g_malloc().

I surveyed the command formats to find a suitable upper bound, but there
is no common protocol limit:

For virgl, Mesa limtis the complete buffer to 260 KiB.

Venus places commands in shared rings, so current Mesa submits
only small control messages directly; its largest fixed outer command
buffer is 256 bytes.

DRM native contexts are more difficult. MSM, i915, and Asahi request
sizes grow with the number of BO or resource records, while AMDGPU can
aggregate multiple chunks. There is no useful common bound below the
outer 32-bit length, or roughly 4 GiB.

I think QEMU needs a practical policy limit, as it already has
for the number of virtio descriptors. A possible limit is 4 MiB.

For vmalloc-backed command data, Linux creates one scatterlist entry per
page, in addition to the command and response entries. With 4 KiB pages
and QEMU's limit of 1,024 mapped iov entries, the largest such command 
is: 1022 * 4 KiB = 4 MiB - 8 KiB

So the existing Linux/QEMU path already stops reliably carrying larger 
inline commands at roughly 4 MiB.

> 
> Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
> 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>
> ---
>   hw/display/virtio-gpu-virgl.c | 11 +++++++++++
>   1 file changed, 11 insertions(+)
> 
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index 60c78af06a4..2a07dd2431f 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -601,12 +601,23 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
>                                   struct virtio_gpu_ctrl_command *cmd)
>   {
>       struct virtio_gpu_cmd_submit cs;
> +    size_t iov_len;
>       void *buf;
>       size_t s;
>   
>       VIRTIO_GPU_FILL_CMD(cs);
>       trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
>   
> +    iov_len = iov_size(cmd->elem.out_sg, cmd->elem.out_num);
> +    if (cs.size == 0 || iov_len < sizeof(cs) ||

A zero-length payload is valid for a fence-only submission, and Mesa
Venus emits such submissions, so cs.size == 0 should not be rejected.

Regards,
Akihiko Odaki

> +        cs.size > iov_len - sizeof(cs)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: size out of range (%u/%zu)",
> +                      __func__, cs.size, iov_len);
> +        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);
Re: [PATCH] hw/display/virtio-gpu: validate command buffer size in submit_3d
Posted by Marc-André Lureau 1 week, 2 days ago 
Hi

On Thu, Jul 16, 2026 at 10:01 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> On 2026/07/13 21:54, marcandre.lureau@redhat.com wrote:
> > From: Marc-André Lureau <marcandre.lureau@redhat.com>
> >
> > virgl_cmd_submit_3d() passes the guest-controlled cs.size directly to
> > g_malloc() without any bounds check. A malicious guest can set this
> > field to an arbitrarily large value (up to 4GB), causing an OOM abort
> > that crashes the QEMU process.
>
> virtio-gpu-rutabaga has the same problem: it also makes a fatal
> guest-sized allocation before copying the payload.

good catch

>
> >
> > Validate cs.size against the actual descriptor payload size before
> > allocating, rejecting values that exceed what the virtqueue entry
> > can carry.
>
> Unfortunately, this check does not bound the allocation. The descriptor
> payload length is also controlled by the guest, and descriptors can
> alias the same guest-physical range. For example, an indirect descriptor
> table can repeat the following entry: addr = A, len = 4 MiB
>
> With 1,023 such entries and one response descriptor, iov_size() reports
> 4092 MiB even though the entries refer to only 4 MiB of unique guest
> memory. A cs.size of nearly 4 GiB still passes the proposed
> check and reaches g_malloc().
>
> I surveyed the command formats to find a suitable upper bound, but there
> is no common protocol limit:
>
> For virgl, Mesa limtis the complete buffer to 260 KiB.
>
> Venus places commands in shared rings, so current Mesa submits
> only small control messages directly; its largest fixed outer command
> buffer is 256 bytes.
>
> DRM native contexts are more difficult. MSM, i915, and Asahi request
> sizes grow with the number of BO or resource records, while AMDGPU can
> aggregate multiple chunks. There is no useful common bound below the
> outer 32-bit length, or roughly 4 GiB.
>
> I think QEMU needs a practical policy limit, as it already has
> for the number of virtio descriptors. A possible limit is 4 MiB.
>
> For vmalloc-backed command data, Linux creates one scatterlist entry per
> page, in addition to the command and response entries. With 4 KiB pages
> and QEMU's limit of 1,024 mapped iov entries, the largest such command
> is: 1022 * 4 KiB = 4 MiB - 8 KiB
>
> So the existing Linux/QEMU path already stops reliably carrying larger
> inline commands at roughly 4 MiB.

Ok, let's start with that. If it's problematic, we can revisit.

>
> >
> > Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
> > 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>
> > ---
> >   hw/display/virtio-gpu-virgl.c | 11 +++++++++++
> >   1 file changed, 11 insertions(+)
> >
> > diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> > index 60c78af06a4..2a07dd2431f 100644
> > --- a/hw/display/virtio-gpu-virgl.c
> > +++ b/hw/display/virtio-gpu-virgl.c
> > @@ -601,12 +601,23 @@ static void virgl_cmd_submit_3d(VirtIOGPU *g,
> >                                   struct virtio_gpu_ctrl_command *cmd)
> >   {
> >       struct virtio_gpu_cmd_submit cs;
> > +    size_t iov_len;
> >       void *buf;
> >       size_t s;
> >
> >       VIRTIO_GPU_FILL_CMD(cs);
> >       trace_virtio_gpu_cmd_ctx_submit(cs.hdr.ctx_id, cs.size);
> >
> > +    iov_len = iov_size(cmd->elem.out_sg, cmd->elem.out_num);
> > +    if (cs.size == 0 || iov_len < sizeof(cs) ||
>
> A zero-length payload is valid for a fence-only submission, and Mesa
> Venus emits such submissions, so cs.size == 0 should not be rejected.

ok

>
> Regards,
> Akihiko Odaki
>
> > +        cs.size > iov_len - sizeof(cs)) {
> > +        qemu_log_mask(LOG_GUEST_ERROR,
> > +                      "%s: size out of range (%u/%zu)",
> > +                      __func__, cs.size, iov_len);
> > +        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);
>
>

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.