Rejecting it in the macros does sound better. For vhost-user-gpu we could still need the patch in vg_handle_ctrl() though. I can send a v3 patch based on this soon.
Thanks, Ankur On Sat, Aug 1, 2026 at 12:42 PM Akihiko Odaki <[email protected]> wrote: > On 2026/07/29 16:28, Ankur Saini wrote: > > Thanks. Just to clarify, the vhost-user-gpu check was already there and > > compares the return value of iov_to_buf(), not iov_size(). != and < > > should be equivalent for vhost-user-gpu check so I left it unchanged. > > The fact that the checks compare different values made me look at how > virtio-gpu copies commands. > > For commands with type-specific fields, processing copies the request > in two stages: > > 1. Copy the common header to determine the command type. > 2. Copy the complete type-specific command. > > vhost-user-gpu open-codes the first step in vg_handle_ctrl() and uses > VUGPU_FILL_CMD() for the second. virtio-gpu uses VIRTIO_GPU_FILL_CMD() > for both steps. > > Both fill macros already detect a short copy, but only log and return. > They should instead complete the malformed request with > ERR_INVALID_PARAMETER. For a common-header copy, the partially copied > header must also be cleared before forming the response. > > Once VIRTIO_GPU_FILL_CMD() handles this failure, the new check in > virtio_gpu_process_cmdq() will be unnecessary because the macro covers > the first step. Fixing both macros will also reject truncated > type-specific commands. > > Regards, > Akihiko Odaki > > > > > The new virtio-gpu check looks at the total iov size, which can be > > larger than the header for valid commands, so it needs to use <. Would > > you still prefer changing the vhost-user-gpu condition to < for > consistency? > > > > Regards, > > Ankur Saini > > > > On Wed, Jul 29, 2026 at 9:31 AM Akihiko Odaki <[email protected] > > tokyo.ac.jp <mailto:[email protected]>> wrote: > > > > 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 > > <https://gitlab.com/qemu-project/qemu/-/work_items/4094> > > > Reported-by: Ankur Saini <[email protected] > > <mailto:[email protected]>> > > > Suggested-by: Akihiko Odaki <[email protected] > > <mailto:[email protected]>> > > > Signed-off-by: Ankur Saini <[email protected] > > <mailto:[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/20260728-virtio- > > [email protected] <https:// > > lore.kernel.org/qemu-devel/20260728-virtio-gpu-short-header-v1-1- > > [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, > > > >
