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

Ankur Saini <[email protected]>
Newsgroups gmane.comp.emulators.qemu
Message-ID <CAE+rwOMBiusrWqbWFgA6Tqxi1pvE4KWEcJkmmw3+0RUYHNiGaQ@mail.gmail.com>
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 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]>
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
> > 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,
>
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.