Re: [PATCH] hw/display/virtio-gpu: Always reject invalid scanout bounds

Ccing Marc-André Lureau

On 2026/08/03 17:45, Akihiko Odaki wrote:
> virtio-gpu does not consistently check scanout bounds with wraparound
> handling. In the unchecked virgl SET_SCANOUT path, guest dimensions
> reach qemu_console_resize(), qemu_create_displaysurface(), and
> ultimately qemu_pixman_image_new_shareable(..., &error_abort), so an
> invalid rectangle can terminate QEMU. Implement a check with proper
> wraparound handling and apply it consistently.
> 
> Fixes: 9d9e152136bd ("virtio-gpu: add 3d mode and virgl rendering support.")
> Fixes: 32db3c63ae11 ("virtio-gpu: Add virtio_gpu_set_scanout_blob")
> Fixes: 7c092f17ccee ("virtio-gpu: Handle resource blob commands")
> Fixes: 1dcc6adbc168 ("gfxstream + rutabaga: add initial support for gfxstream")
> Signed-off-by: Akihiko Odaki <[email protected]>
> ---
>   include/hw/virtio/virtio-gpu.h   |  5 +++++
>   hw/display/virtio-gpu-rutabaga.c |  6 ++++++
>   hw/display/virtio-gpu-virgl.c    | 20 +++++++++-----------
>   hw/display/virtio-gpu.c          | 36 ++++++++++++++++++++++--------------
>   4 files changed, 42 insertions(+), 25 deletions(-)
> 
> diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
> index 2f60c72078b3..287262228d33 100644
> --- a/include/hw/virtio/virtio-gpu.h
> +++ b/include/hw/virtio/virtio-gpu.h
> @@ -363,6 +363,11 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
>                                      struct virtio_gpu_scanout *s,
>                                      uint32_t resource_id);
>   
> +bool virtio_gpu_check_scanout_bounds(uint32_t scanout_id, uint32_t resource_id,
> +                                     uint32_t width, uint32_t height,
> +                                     const struct virtio_gpu_rect *r,
> +                                     uint32_t *error);
> +
>   /**
>    * virtio_gpu_scanout_blob_to_fb() - fill out fb based on scanout data
>    * fb: the frame-buffer descriptor to fill out
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index e28aad94eead..a054f8117f14 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -315,6 +315,12 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
>       res = virtio_gpu_find_resource(g, ss.resource_id);
>       CHECK(res, cmd);
>   
> +    if (!virtio_gpu_check_scanout_bounds(ss.scanout_id, ss.resource_id,
> +                                         res->width, res->height, &ss.r,
> +                                         &cmd->error)) {
> +        return;
> +    }
> +
>       if (!res->image) {
>           pixman_format_code_t pformat;
>           pformat = virtio_gpu_get_pixman_format(res->format);
> diff --git a/hw/display/virtio-gpu-virgl.c b/hw/display/virtio-gpu-virgl.c
> index d9e5b0110497..6e298f997d66 100644
> --- a/hw/display/virtio-gpu-virgl.c
> +++ b/hw/display/virtio-gpu-virgl.c
> @@ -560,7 +560,7 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
>       }
>       g->parent_obj.enable = 1;
>   
> -    if (ss.resource_id && ss.r.width && ss.r.height) {
> +    if (ss.resource_id) {
>           struct virgl_renderer_resource_info info;
>           void *d3d_tex2d = NULL;
>   
> @@ -581,6 +581,11 @@ static void virgl_cmd_set_scanout(VirtIOGPU *g,
>               cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_RESOURCE_ID;
>               return;
>           }
> +        if (!virtio_gpu_check_scanout_bounds(ss.scanout_id, ss.resource_id,
> +                                             info.width, info.height, &ss.r,
> +                                             &cmd->error)) {
> +            return;
> +        }
>           qemu_console_resize(g->parent_obj.scanout[ss.scanout_id].con,
>                               ss.r.width, ss.r.height);
>           virgl_renderer_force_ctx_0();
> @@ -987,16 +992,9 @@ static void virgl_cmd_set_scanout_blob(VirtIOGPU *g,
>           return;
>       }
>   
> -    if (ss.width < 16 ||
> -        ss.height < 16 ||
> -        ss.r.x + ss.r.width > ss.width ||
> -        ss.r.y + ss.r.height > ss.height) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
> -                      " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
> -                      __func__, ss.scanout_id, ss.resource_id,
> -                      ss.r.x, ss.r.y, ss.r.width, ss.r.height,
> -                      ss.width, ss.height);
> -        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +    if (!virtio_gpu_check_scanout_bounds(ss.scanout_id, ss.resource_id,
> +                                         ss.width, ss.height, &ss.r,
> +                                         &cmd->error)) {
>           return;
>       }
>   
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 4d46a4eb10fa..3c0e38c1def0 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -622,6 +622,26 @@ static uint32_t virtio_gpu_format_bytes_pp(pixman_format_code_t format)
>       return DIV_ROUND_UP(PIXMAN_FORMAT_BPP(format), 8);
>   }
>   
> +bool virtio_gpu_check_scanout_bounds(uint32_t scanout_id, uint32_t resource_id,
> +                                     uint32_t width, uint32_t height,
> +                                     const struct virtio_gpu_rect *r,
> +                                     uint32_t *error)
> +{
> +    if (r->width < 16 ||
> +        r->height < 16 ||
> +        (uint64_t)r->x + r->width > width ||
> +        (uint64_t)r->y + r->height > height) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
> +                      " resource %d, fb %d %d, rect (%d,%d)+%d,%d\n",
> +                      __func__, scanout_id, resource_id, width, height,
> +                      r->x, r->y, r->width, r->height);
> +        *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +        return false;
> +    }
> +
> +    return true;
> +}
> +
>   static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
>                                         uint32_t scanout_id,
>                                         struct virtio_gpu_framebuffer *fb,
> @@ -635,20 +655,8 @@ static bool virtio_gpu_do_set_scanout(VirtIOGPU *g,
>   
>       scanout = &g->parent_obj.scanout[scanout_id];
>   
> -    if (r->x > fb->width ||
> -        r->y > fb->height ||
> -        r->width < 16 ||
> -        r->height < 16 ||
> -        r->width > fb->width ||
> -        r->height > fb->height ||
> -        r->x + r->width > fb->width ||
> -        r->y + r->height > fb->height) {
> -        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout %d bounds for"
> -                      " resource %d, rect (%d,%d)+%d,%d, fb %d %d\n",
> -                      __func__, scanout_id, res->resource_id,
> -                      r->x, r->y, r->width, r->height,
> -                      fb->width, fb->height);
> -        *error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER;
> +    if (!virtio_gpu_check_scanout_bounds(scanout_id, res->resource_id,
> +                                         fb->width, fb->height, r, error)) {
>           return false;
>       }
>   
> 
> ---
> base-commit: 299e7557ed15a9a325620698add379a3ce2d1d95
> change-id: 20260803-scanout-c7cf28c1890e
> 
> Best regards,
> --
> Akihiko Odaki <[email protected]>
>