hw/display/virtio-gpu: Protect from DMA re-entrancy bugs

Replace qemu_bh_new_guarded() by virtio_bh_new_guarded()
so the bus and device use the same guard. Otherwise the
DMA-reentrancy protection ca

hw/display/virtio-gpu: Protect from DMA re-entrancy bugs

Replace qemu_bh_new_guarded() by virtio_bh_new_guarded()
so the bus and device use the same guard. Otherwise the
DMA-reentrancy protection can be bypassed:

$ cat << EOF | qemu-system-i386 -display none -nodefaults \
-machine q35,accel=qtest \
-m 512M \
-device virtio-gpu \
-qtest stdio
outl 0xcf8 0x80000820
outl 0xcfc 0xe0004000
outl 0xcf8 0x80000804
outw 0xcfc 0x06
write 0xe0004030 0x4 0x024000e0
write 0xe0004028 0x1 0xff
write 0xe0004020 0x4 0x00009300
write 0xe000401c 0x1 0x01
write 0x101 0x1 0x04
write 0x103 0x1 0x1c
write 0x9301c8 0x1 0x18
write 0x105 0x1 0x1c
write 0x107 0x1 0x1c
write 0x109 0x1 0x1c
write 0x10b 0x1 0x00
write 0x10d 0x1 0x00
write 0x10f 0x1 0x00
write 0x111 0x1 0x00
write 0x113 0x1 0x00
write 0x115 0x1 0x00
write 0x117 0x1 0x00
write 0x119 0x1 0x00
write 0x11b 0x1 0x00
write 0x11d 0x1 0x00
write 0x11f 0x1 0x00
write 0x121 0x1 0x00
write 0x123 0x1 0x00
write 0x125 0x1 0x00
write 0x127 0x1 0x00
write 0x129 0x1 0x00
write 0x12b 0x1 0x00
write 0x12d 0x1 0x00
write 0x12f 0x1 0x00
write 0x131 0x1 0x00
write 0x133 0x1 0x00
write 0x135 0x1 0x00
write 0x137 0x1 0x00
write 0x139 0x1 0x00
write 0xe0007003 0x1 0x00
EOF
...
=================================================================
==276099==ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000011178
at pc 0x562cc3b736c7 bp 0x7ffed49dee60 sp 0x7ffed49dee58
READ of size 8 at 0x60d000011178 thread T0
#0 0x562cc3b736c6 in virtio_gpu_ctrl_response hw/display/virtio-gpu.c:180:42
#1 0x562cc3b7c40b in virtio_gpu_ctrl_response_nodata hw/display/virtio-gpu.c:192:5
#2 0x562cc3b7c40b in virtio_gpu_simple_process_cmd hw/display/virtio-gpu.c:1015:13
#3 0x562cc3b82873 in virtio_gpu_process_cmdq hw/display/virtio-gpu.c:1050:9
#4 0x562cc4a85514 in aio_bh_call util/async.c:169:5
#5 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13
#6 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5
#7 0x562cc4a8a2da in aio_ctx_dispatch util/async.c:358:5
#8 0x7f36840547a8 in g_main_context_dispatch (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x547a8)
#9 0x562cc4a8b753 in glib_pollfds_poll util/main-loop.c:290:9
#10 0x562cc4a8b753 in os_host_main_loop_wait util/main-loop.c:313:5
#11 0x562cc4a8b753 in main_loop_wait util/main-loop.c:592:11
#12 0x562cc3938186 in qemu_main_loop system/runstate.c:782:9
#13 0x562cc43b7af5 in qemu_default_main system/main.c:37:14
#14 0x7f3683a6c189 in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#15 0x7f3683a6c244 in __libc_start_main csu/../csu/libc-start.c:381:3
#16 0x562cc2a58ac0 in _start (qemu-system-i386+0x231bac0)

0x60d000011178 is located 56 bytes inside of 136-byte region [0x60d000011140,0x60d0000111c8)
freed by thread T0 here:
#0 0x562cc2adb662 in __interceptor_free (qemu-system-i386+0x239e662)
#1 0x562cc3b86b21 in virtio_gpu_reset hw/display/virtio-gpu.c:1524:9
#2 0x562cc416e20e in virtio_reset hw/virtio/virtio.c:2145:9
#3 0x562cc37c5644 in virtio_pci_reset hw/virtio/virtio-pci.c:2249:5
#4 0x562cc4233758 in memory_region_write_accessor system/memory.c:497:5
#5 0x562cc4232eea in access_with_adjusted_size system/memory.c:573:18

previously allocated by thread T0 here:
#0 0x562cc2adb90e in malloc (qemu-system-i386+0x239e90e)
#1 0x7f368405a678 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x5a678)
#2 0x562cc4163ffc in virtqueue_split_pop hw/virtio/virtio.c:1612:12
#3 0x562cc4163ffc in virtqueue_pop hw/virtio/virtio.c:1783:16
#4 0x562cc3b91a95 in virtio_gpu_handle_ctrl hw/display/virtio-gpu.c:1112:15
#5 0x562cc4a85514 in aio_bh_call util/async.c:169:5
#6 0x562cc4a85c52 in aio_bh_poll util/async.c:216:13
#7 0x562cc4a1a79b in aio_dispatch util/aio-posix.c:423:5

SUMMARY: AddressSanitizer: heap-use-after-free hw/display/virtio-gpu.c:180:42 in virtio_gpu_ctrl_response

With this change, the same reproducer triggers:

qemu-system-i386: warning: Blocked re-entrant IO on MemoryRegion: virtio-pci-common-virtio-gpu at addr: 0x6

Fixes: CVE-2024-3446
Cc: qemu-stable@nongnu.org
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Reported-by: Yongkang Jia <kangel@zju.edu.cn>
Reported-by: Xiao Lei <nop.leixiao@gmail.com>
Reported-by: Yiming Tao <taoym@zju.edu.cn>
Buglink: https://bugs.launchpad.net/qemu/+bug/1888606
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-Id: <20240409105537.18308-3-philmd@linaro.org>

show more ...

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migratio

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migration support in commit f66767f75c9, I didn't realize that blob
resources could be used for scanouts. This situationn leads to a crash
during post-load, as they don't have an associated res->image.

virtio_gpu_do_set_scanout() handle all cases, but requires the
associated virtio_gpu_framebuffer, which is currently not saved during
migration.

Add a v2 of "virtio-gpu-one-scanout" with the framebuffer fields, so we
can restore blob scanouts, as well as fixing the existing FIXME.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Sebastian Ott <sebott@redhat.com>

show more ...

virtio-gpu: remove needless condition

qemu_create_displaysurface_pixman() never returns NULL.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migratio

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migration support in commit f66767f75c9, I didn't realize that blob
resources could be used for scanouts. This situationn leads to a crash
during post-load, as they don't have an associated res->image.

virtio_gpu_do_set_scanout() handle all cases, but requires the
associated virtio_gpu_framebuffer, which is currently not saved during
migration.

Add a v2 of "virtio-gpu-one-scanout" with the framebuffer fields, so we
can restore blob scanouts, as well as fixing the existing FIXME.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Sebastian Ott <sebott@redhat.com>

show more ...

virtio-gpu: remove needless condition

qemu_create_displaysurface_pixman() never returns NULL.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migratio

virtio-gpu: fix scanout migration post-load

The current post-loading code for scanout has a FIXME: it doesn't take
the resource region/rect into account. But there is more, when adding
blob migration support in commit f66767f75c9, I didn't realize that blob
resources could be used for scanouts. This situationn leads to a crash
during post-load, as they don't have an associated res->image.

virtio_gpu_do_set_scanout() handle all cases, but requires the
associated virtio_gpu_framebuffer, which is currently not saved during
migration.

Add a v2 of "virtio-gpu-one-scanout" with the framebuffer fields, so we
can restore blob scanouts, as well as fixing the existing FIXME.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Sebastian Ott <sebott@redhat.com>

show more ...

virtio-gpu: remove needless condition

qemu_create_displaysurface_pixman() never returns NULL.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

virtio-gpu.c: add resource_destroy class method

When destroying/unrefing resources, devices such as virtio-gpu-rutabaga
need to do their own bookkeeping (free rutabaga resources that are
associated

virtio-gpu.c: add resource_destroy class method

When destroying/unrefing resources, devices such as virtio-gpu-rutabaga
need to do their own bookkeeping (free rutabaga resources that are
associated with the virtio_gpu_simple_resource).

This commit adds a class method so that virtio-gpu-rutabaga can override
it in the next commit.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Message-Id: <b0a86630c4d601f3a269fd7e08cfefc13bd4e219.1706626470.git.manos.pitsidianakis@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

show more ...

hw/display/virtio-gpu.c: use reset_bh class method

While the VirtioGPU type has a reset_bh field to specify a reset
callback, it's never used. virtio_gpu_reset() calls the general
virtio_gpu_reset_b

hw/display/virtio-gpu.c: use reset_bh class method

While the VirtioGPU type has a reset_bh field to specify a reset
callback, it's never used. virtio_gpu_reset() calls the general
virtio_gpu_reset_bh() function for all devices that inherit from
VirtioGPU.

While no devices override reset_bh at the moment, a device reset might
require special logic for implementations in the future.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Message-Id: <87fb4fa72ce5b341a6f957513a00dcb79fd5997f.1706626470.git.manos.pitsidianakis@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

show more ...

qemu/main-loop: rename qemu_cond_wait_iothread() to qemu_cond_wait_bql()

The name "iothread" is overloaded. Use the term Big QEMU Lock (BQL)
instead, it is already widely used and unambiguous.

Sign

qemu/main-loop: rename qemu_cond_wait_iothread() to qemu_cond_wait_bql()

The name "iothread" is overloaded. Use the term Big QEMU Lock (BQL)
instead, it is already widely used and unambiguous.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Paul Durrant <paul@xen.org>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>
Message-id: 20240102153529.486531-4-stefanha@redhat.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>

show more ...

hw/display: Constify VMState

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20231221031652.119827-27-richard.henderson@linaro.org>

virtio-gpu: replace PIXMAN for region/rect test

Use a simpler implementation for rectangle geometry & intersect, drop
the need for (more complex) PIXMAN functions.

Signed-off-by: Marc-André Lureau

virtio-gpu: replace PIXMAN for region/rect test

Use a simpler implementation for rectangle geometry & intersect, drop
the need for (more complex) PIXMAN functions.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>

show more ...

Revert "virtio-gpu: block migration of VMs with blob=true"

If we decide to apply this patch (for easier backporting reasons), we
can now revert it.

Signed-off-by: Marc-André Lureau <marcandre.lurea

Revert "virtio-gpu: block migration of VMs with blob=true"

If we decide to apply this patch (for easier backporting reasons), we
can now revert it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: add virtio-gpu/blob vmstate subsection

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

virtio-gpu: move scanout restoration to post_load

As we are going to introduce an extra subsection for "blob" resources,
scanout have to be restored after.

Signed-off-by: Marc-André Lureau <marcand

virtio-gpu: move scanout restoration to post_load

As we are going to introduce an extra subsection for "blob" resources,
scanout have to be restored after.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: factor out restore mapping

The same function is going to be used next to restore "blob" resources.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <pet

virtio-gpu: factor out restore mapping

The same function is going to be used next to restore "blob" resources.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: block migration of VMs with blob=true

"blob" resources don't have an associated pixman image:

#0 pixman_image_get_stride (image=0x0) at ../pixman/pixman-image.c:921
#1 0x0000562327c25

virtio-gpu: block migration of VMs with blob=true

"blob" resources don't have an associated pixman image:

#0 pixman_image_get_stride (image=0x0) at ../pixman/pixman-image.c:921
#1 0x0000562327c25236 in virtio_gpu_save (f=0x56232bb13b00, opaque=0x56232b555a60, size=0, field=0x5623289ab6c8 <__compound_literal.3+104>, vmdesc=0x56232ab59fe0) at ../hw/display/virtio-gpu.c:1225

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=2236353

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

Revert "virtio-gpu: block migration of VMs with blob=true"

If we decide to apply this patch (for easier backporting reasons), we
can now revert it.

Signed-off-by: Marc-André Lureau <marcandre.lurea

Revert "virtio-gpu: block migration of VMs with blob=true"

If we decide to apply this patch (for easier backporting reasons), we
can now revert it.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: add virtio-gpu/blob vmstate subsection

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

virtio-gpu: move scanout restoration to post_load

As we are going to introduce an extra subsection for "blob" resources,
scanout have to be restored after.

Signed-off-by: Marc-André Lureau <marcand

virtio-gpu: move scanout restoration to post_load

As we are going to introduce an extra subsection for "blob" resources,
scanout have to be restored after.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: factor out restore mapping

The same function is going to be used next to restore "blob" resources.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <pet

virtio-gpu: factor out restore mapping

The same function is going to be used next to restore "blob" resources.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

virtio-gpu: block migration of VMs with blob=true

"blob" resources don't have an associated pixman image:

#0 pixman_image_get_stride (image=0x0) at ../pixman/pixman-image.c:921
#1 0x0000562327c25

virtio-gpu: block migration of VMs with blob=true

"blob" resources don't have an associated pixman image:

#0 pixman_image_get_stride (image=0x0) at ../pixman/pixman-image.c:921
#1 0x0000562327c25236 in virtio_gpu_save (f=0x56232bb13b00, opaque=0x56232b555a60, size=0, field=0x5623289ab6c8 <__compound_literal.3+104>, vmdesc=0x56232ab59fe0) at ../hw/display/virtio-gpu.c:1225

Related to:
https://bugzilla.redhat.com/show_bug.cgi?id=2236353

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Peter Xu <peterx@redhat.com>

show more ...

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirk

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20231017140150.44995-4-philmd@linaro.org>

show more ...

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirk

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20231017140150.44995-4-philmd@linaro.org>

show more ...

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirk

hw/display/virtio-gpu: Use VIRTIO_DEVICE() macro

Access QOM parent with the proper QOM VIRTIO_DEVICE() macro.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20231017140150.44995-4-philmd@linaro.org>

show more ...