Re: [PATCH] hw/display/virtio-gpu: Always reject invalid scanout bounds
Akihiko Odaki <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <[email protected]> |
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]>
>