hw/display/virtio-gpu-rutabaga.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
From: Haotian Jiang <sundayjiang@tencent.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.
Replace VIRTIO_GPU_MAX_SCANOUTS with vb->conf.max_outputs in the
CHECK, since realization already ensures max_outputs <=
VIRTIO_GPU_MAX_SCANOUTS.
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 | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
index 6ff1263901..3f78899442 100644
--- a/hw/display/virtio-gpu-rutabaga.c
+++ b/hw/display/virtio-gpu-rutabaga.c
@@ -302,7 +302,7 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
ss.r.width, ss.r.height, ss.r.x, ss.r.y);
- CHECK(ss.scanout_id < VIRTIO_GPU_MAX_SCANOUTS, cmd);
+ CHECK(ss.scanout_id < vb->conf.max_outputs, cmd);
scanout = &vb->scanout[ss.scanout_id];
if (ss.resource_id == 0) {
--
Changes v1 -> v2:
- Replace VIRTIO_GPU_MAX_SCANOUTS with vb->conf.max_outputs per
Akihiko Odaki's suggestion (simpler, no extra if block needed)
2.34.1
Hi,
The change itself looks good, but there are several nits in the patch
format.
First, please ensure you to describe the version of patch series. For
example, this patch's subject should be:
"[PATCH v2] virtio-gpu: fix NULL deref in rutabaga set_scanout"
Please follow docs/devel/submitting-a-patch.rst to get the correct
subject (and also to find hints and tools that address troubles with
patch submission).
On 2026/07/20 11:21, jianghaotian.sunday@gmail.com wrote:
> From: Haotian Jiang <sundayjiang@tencent.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.
scanout_id >= VIRTIO_GPU_MAX_SCANOUTS is already covered by an existing
check. Crashing QEMU may be the most probable consequence, but it may
turn out otherwise. I think just noting the out of bound access is
sufficient and concise.
>
> Replace VIRTIO_GPU_MAX_SCANOUTS with vb->conf.max_outputs in the
> CHECK, since realization already ensures max_outputs <=
> VIRTIO_GPU_MAX_SCANOUTS.
>
> Fixes: 1dcc6adbc1 ("gfxstream + rutabaga: add initial support for gfxstream")
docs/devel/submitting-a-patch.rst says:
> If your patch fixes a commit that is already in the repository, please
> add an additional line with
> "Fixes: <at-least-12-digits-of-SHA-commit-id>
> ("Fixed commit subject")" below the patch description / before your
> "Signed-off-by:" line in the commit message.
Unfortunately, QEMU's documentation doesn't tell much about how to
generate the Fixes: tag, but Linux's documentation has more details:
https://www.kernel.org/doc/html/v7.1/process/submitting-patches.html#describe-your-changes
> Reported-by: Haotian Jiang of Tencent Security (Yunding Lab) <jianghaotian.sunday@gmail.com>
> Signed-off-by: Haotian Jiang <jianghaotian.sunday@gmail.com>
docs/devel/code-provenance.rst says:
> It is generally expected that the name and email addresses used in one
> of the ``Signed-off-by`` lines, matches that of the git commit
> ``Author`` field. It's okay if you subscribe or contribute to the list
> via more than one address, but using multiple addresses in one commit
> just confuses things.
Regards,
Akihiko Odaki
> Cc: qemu-stable@nongnu.org
> ---
> hw/display/virtio-gpu-rutabaga.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index 6ff1263901..3f78899442 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -302,7 +302,7 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
> trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
> ss.r.width, ss.r.height, ss.r.x, ss.r.y);
>
> - CHECK(ss.scanout_id < VIRTIO_GPU_MAX_SCANOUTS, cmd);
> + CHECK(ss.scanout_id < vb->conf.max_outputs, cmd);
> scanout = &vb->scanout[ss.scanout_id];
>
> if (ss.resource_id == 0) {
On 7/20/26 05:21, jianghaotian.sunday@gmail.com wrote:
> From: Haotian Jiang <sundayjiang@tencent.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.
>
> Replace VIRTIO_GPU_MAX_SCANOUTS with vb->conf.max_outputs in the
> CHECK, since realization already ensures max_outputs <=
> VIRTIO_GPU_MAX_SCANOUTS.
>
> 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 | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/display/virtio-gpu-rutabaga.c b/hw/display/virtio-gpu-rutabaga.c
> index 6ff1263901..3f78899442 100644
> --- a/hw/display/virtio-gpu-rutabaga.c
> +++ b/hw/display/virtio-gpu-rutabaga.c
> @@ -302,7 +302,7 @@ rutabaga_cmd_set_scanout(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd)
> trace_virtio_gpu_cmd_set_scanout(ss.scanout_id, ss.resource_id,
> ss.r.width, ss.r.height, ss.r.x, ss.r.y);
>
> - CHECK(ss.scanout_id < VIRTIO_GPU_MAX_SCANOUTS, cmd);
> + CHECK(ss.scanout_id < vb->conf.max_outputs, cmd);
> scanout = &vb->scanout[ss.scanout_id];
>
> if (ss.resource_id == 0) {
Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
--
Best regards,
Dmitry
© 2016 - 2026 Red Hat, Inc.