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.
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: [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.
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);
