Re: [PATCH v3] virtio-gpu: reject requests with short/truncated control headers

Marc-AndrĂ© Lureau <[email protected]> Tue, 4 Aug 2026 11:28:55 +0400
Newsgroups gmane.comp.emulators.qemu
Message-ID <CAJ+F1CK-cfC0zfZEEkdhuVdNwsuLQ9tLqAts-R9==voGVOyMBg@mail.gmail.com>
Hi

On Tue, Aug 4, 2026 at 9:59=E2=80=AFAM Akihiko Odaki
<[email protected]> wrote:
>
> On 2026/08/04 2:09, Ankur Saini wrote:
> > A short control request can leave command data partially initialized.
> > For the common header, guest-controlled flags can then cause stale fenc=
e
> > metadata to be returned to the guest.
> >
> > The command fill helpers detect a short copy but only log and return,
> > leaving the malformed request without an error or completion. Make
> > VIRTIO_GPU_FILL_CMD() clear the partially copied object and complete th=
e
> > request with ERR_INVALID_PARAMETER. Make VUGPU_FILL_CMD() report the sa=
me
> > error through the existing vhost-user-gpu dispatcher. This also rejects
> > truncated type-specific commands.
>
> One small correction: this says the malformed request is left without
> completion, but virtio_gpu_simple_process_cmd() and vg_process_cmd()
> actually complete them, though its response type is
> VIRTIO_GPU_RESP_OK_NODATA.
>
> With that addressed,
>
> Reviewed-by: Akihiko Odaki <[email protected]>

I queued this patch and updated the commit message
Reviewed-by: Marc-Andr=C3=A9 Lureau <[email protected]>

>
> Regards,
> Akihiko Odaki
>
> >
> > The vhost-user-gpu common header is copied outside VUGPU_FILL_CMD(), so
> > clear it and complete the request directly when that copy is short.
> >
> > 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 v3:
> > - Handle short copies in VIRTIO_GPU_FILL_CMD() and VUGPU_FILL_CMD(),
> >    also rejecting truncated type-specific commands.
> > - Clear partially copied QEMU command data before completing the error.
> > - Keep vhost-user-gpu's explicit common-header check and drop the
> >    redundant virtio_gpu_process_cmdq() check.
> > - Rebase onto current master.
> > - Link to v2: https://lore.kernel.org/qemu-devel/20260729-virtio-gpu-sh=
[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-gpu-sh=
[email protected]
> > ---
> >   contrib/vhost-user-gpu/vhost-user-gpu.c | 21 ++++++++++++---------
> >   contrib/vhost-user-gpu/vugpu.h          |  1 +
> >   include/hw/virtio/virtio-gpu.h          |  3 +++
> >   3 files changed, 16 insertions(+), 9 deletions(-)
> >
> > diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-us=
er-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 !=3D 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 !=3D VG_CMD_STATE_FINISHED) {
> > diff --git a/contrib/vhost-user-gpu/vugpu.h b/contrib/vhost-user-gpu/vu=
gpu.h
> > index 2374eb90cb..aaf2870cb2 100644
> > --- a/contrib/vhost-user-gpu/vugpu.h
> > +++ b/contrib/vhost-user-gpu/vugpu.h
> > @@ -179,6 +179,7 @@ struct virtio_gpu_ctrl_command {
> >           if (vugpufillcmd_s_ !=3D sizeof(out)) {                   \
> >               g_critical("%s: command size incorrect %zu vs %zu", \
> >                          __func__, vugpufillcmd_s_, sizeof(out)); \
> > +            cmd->error =3D VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; \
> >               return;                                             \
> >           }                                                       \
> >       } while (0)
> > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-=
gpu.h
> > index 2f60c72078..f965defa6b 100644
> > --- a/include/hw/virtio/virtio-gpu.h
> > +++ b/include/hw/virtio/virtio-gpu.h
> > @@ -315,6 +315,9 @@ struct VirtIOGPURutabaga {
> >               qemu_log_mask(LOG_GUEST_ERROR,                           =
   \
> >                             "%s: command size incorrect %zu vs %zu\n", =
   \
> >                             __func__, virtiogpufillcmd_s_, sizeof(out))=
;  \
> > +            memset(&out, 0, sizeof(out));                             =
  \
> > +            virtio_gpu_ctrl_response_nodata(                          =
  \
> > +                g, cmd, VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER);       =
  \
> >               return;                                                  =
   \
> >           }                                                            =
   \
> >       } while (0)
> >
> > ---
> > base-commit: b428fe036233cbd15d37e3c027ab6ca4d3661a80
> > change-id: 20260728-virtio-gpu-short-header-476aa1dae4f7
> >
> > Best regards,
>
>