Re: [PATCH v2] virtio-gpu: reject requests with short/truncated control headers
Akihiko Odaki <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <[email protected]> |
On 2026/07/29 5:44, Ankur Saini wrote: > A control request shorter than virtio_gpu_ctrl_hdr can leave cmd_hdr > partially initialized. If the supplied bytes set the fence flag, stale > fence metadata may later be returned to the guest. > > The vhost-user-gpu backend has the same issue: it logs a short header > copy but continues to process the partially initialized command. > > Validate the common header length before dispatch in both paths. Clear > cmd_hdr and complete malformed requests with ERR_INVALID_PARAMETER so > stale fields cannot reach the response. vhost-user-gpu and virtio-gpu has slightly different conditions: - vhost-user-gpu rejects: iov_size() != sizeof(cmd->cmd_hdr) - virtio-gpu rejects: iov_size() < sizeof(cmd->cmd_hdr) Ideally they should be consistent. Regards, Akihiko Odaki > > Fixes: CVE-2026-18054 > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4094 > Reported-by: Ankur Saini <[email protected]> > Suggested-by: Akihiko Odaki <[email protected]> > Signed-off-by: Ankur Saini <[email protected]> > --- > Changes in v2: > - Also reject short/truncated control headers in vhost-user-gpu. > - Expand the commit message to describe both affected paths. > - Link to v1: https://lore.kernel.org/qemu-devel/[email protected] > --- > contrib/vhost-user-gpu/vhost-user-gpu.c | 21 ++++++++++++--------- > hw/display/virtio-gpu.c | 10 ++++++++-- > 2 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c > index bb41758e34..8149834745 100644 > --- a/contrib/vhost-user-gpu/vhost-user-gpu.c > +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c > @@ -924,16 +924,19 @@ vg_handle_ctrl(VuDev *dev, int qidx) > if (len != sizeof(cmd->cmd_hdr)) { > g_warning("%s: command size incorrect %zu vs %zu\n", > __func__, len, sizeof(cmd->cmd_hdr)); > - } > - > - virtio_gpu_ctrl_hdr_bswap(&cmd->cmd_hdr); > - g_debug("%d %s\n", cmd->cmd_hdr.type, > - vg_cmd_to_string(cmd->cmd_hdr.type)); > - > - if (vg->virgl) { > - vg_virgl_process_cmd(vg, cmd); > + memset(&cmd->cmd_hdr, 0, sizeof(cmd->cmd_hdr)); > + vg_ctrl_response_nodata( > + vg, cmd, VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER); > } else { > - vg_process_cmd(vg, cmd); > + virtio_gpu_ctrl_hdr_bswap(&cmd->cmd_hdr); > + g_debug("%d %s\n", cmd->cmd_hdr.type, > + vg_cmd_to_string(cmd->cmd_hdr.type)); > + > + if (vg->virgl) { > + vg_virgl_process_cmd(vg, cmd); > + } else { > + vg_process_cmd(vg, cmd); > + } > } > > if (cmd->state != VG_CMD_STATE_FINISHED) { > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 4d46a4eb10..15a845eff6 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -1105,8 +1105,14 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g) > break; > } > > - /* process command */ > - vgc->process_cmd(g, cmd); > + if (unlikely(iov_size(cmd->elem.out_sg, cmd->elem.out_num) < > + sizeof(cmd->cmd_hdr))) { > + memset(&cmd->cmd_hdr, 0, sizeof(cmd->cmd_hdr)); > + virtio_gpu_ctrl_response_nodata( > + g, cmd, VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER); > + } else { > + vgc->process_cmd(g, cmd); > + } > > /* command suspended */ > if (!cmd->finished && !(cmd->cmd_hdr.flags & VIRTIO_GPU_FLAG_FENCE)) { > > --- > base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95 > change-id: 20260728-virtio-gpu-short-header-476aa1dae4f7 > > Best regards,