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

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,
>