Hi
On Thu, Jul 16, 2026 at 10:01 AM Akihiko Odaki
<[email protected]> wrote:
>
> On 2026/07/13 21:54, [email protected] wrote:
> > From: Marc-André Lureau <[email protected]>
> >
> > 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: [email protected]
> > Signed-off-by: Marc-André Lureau <[email protected]>
> > ---
> > 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);
>
>
