[v1] virtio-gpu: fix NULL deref in rutabaga set_scanout

[PATCH] virtio-gpu: fix NULL deref in rutabaga set_scanout

Posted by jianghaotian.sunday@gmail.com 1 week, 1 day ago

From: Haotian Jiang <jianghaotian.sunday@gmail.com>

rutabaga_cmd_set_scanout() checks scanout_id < VIRTIO_GPU_MAX_SCANOUTS
(16), but does not check scanout_id < conf.max_outputs like the base
class (virtio-gpu.c) and virgl backend (virtio-gpu-virgl.c) do.

With the default max_outputs=1, virtio_gpu_base_device_realize only
initializes scanout[0].con. A guest submitting SET_SCANOUT with
scanout_id >= 1 takes the con=NULL path, and
qemu_console_set_surface(NULL, NULL) dereferences con->ds, crashing
QEMU.

Add the same max_outputs check used by the base class and virgl backend.

Fixes: 1dcc6adbc1 ("gfxstream + rutabaga: add initial support for gfxstream")
Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
Cc: qemu-stable@nongnu.org
---
 hw/display/virtio-gpu-rutabaga.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 6ff1263901..24386487ba 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -303,6 +303,14 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
                                      ss.r.width, ss.r.height, ss.r.x, ss.r.y);
 
     CHECK(ss.scanout_id < VIRTIO_GPU_MAX_SCANOUTS, cmd);
+
+    if (ss.scanout_id >= vb->conf.max_outputs) {
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
+                      __func__, ss.scanout_id);
+        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
+        return;
+    }
+
     scanout = &vb->scanout[ss.scanout_id];
 
     if (ss.resource_id == 0) {
-- 
2.34.1

Re: [PATCH] virtio-gpu: fix NULL deref in rutabaga set_scanout

Posted by Akihiko Odaki 1 week, 1 day ago

On 2026/07/17 17:32, jianghaotian.sunday@gmail.com wrote:
> From: Haotian Jiang <jianghaotian.sunday@gmail.com>
> 
> rutabaga_cmd_set_scanout() checks scanout_id < VIRTIO_GPU_MAX_SCANOUTS
> (16), but does not check scanout_id < conf.max_outputs like the base
> class (virtio-gpu.c) and virgl backend (virtio-gpu-virgl.c) do.
> 
> With the default max_outputs=1, virtio_gpu_base_device_realize only
> initializes scanout[0].con. A guest submitting SET_SCANOUT with
> scanout_id >= 1 takes the con=NULL path, and
> qemu_console_set_surface(NULL, NULL) dereferences con->ds, crashing
> QEMU.
> 
> Add the same max_outputs check used by the base class and virgl backend.
> 
> Fixes: 1dcc6adbc1 ("gfxstream + rutabaga: add initial support for gfxstream")
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
> Cc: qemu-stable@nongnu.org
> ---
>   hw/display/virtio-gpu-rutabaga.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index 6ff1263901..24386487ba 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -303,6 +303,14 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
>                                        ss.r.width, ss.r.height, ss.r.x, ss.r.y);
>   
>       CHECK(ss.scanout_id < VIRTIO_GPU_MAX_SCANOUTS, cmd);
> +
> +    if (ss.scanout_id >= vb->conf.max_outputs) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: illegal scanout id specified %d",
> +                      __func__, ss.scanout_id);
> +        cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_SCANOUT_ID;
> +        return;
> +    }
> +

We can simply replace VIRTIO_GPU_MAX_SCANOUTS with vb->conf.max_outputs 
since we already ensure vb->conf.max_outputs <= VIRTIO_GPU_MAX_SCANOUTS 
during realization.

Regards,
Akihiko Odaki

>       scanout = &vb->scanout[ss.scanout_id];
>   
>       if (ss.resource_id == 0) {