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

jianghaotian.sunday@gmail.com posted 1 patch 5 days, 9 hours ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260720022100.3700582-1-jianghaotian.sunday@gmail.com
Maintainers: "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/display/virtio-gpu-rutabaga.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] virtio-gpu: fix NULL deref in rutabaga set_scanout
Posted by jianghaotian.sunday@gmail.com 5 days, 9 hours ago
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
Re: [PATCH] virtio-gpu: fix NULL deref in rutabaga set_scanout
Posted by Akihiko Odaki 5 days, 4 hours ago
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) {
Re: [PATCH] virtio-gpu: fix NULL deref in rutabaga set_scanout
Posted by Dmitry Osipenko 5 days, 5 hours ago
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