Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

vfio/pci: Fix VFIOGroup use after free #14

Open
wants to merge 1 commit into
base: vfio-user-dbfix
Choose a base branch
from
Open

vfio/pci: Fix VFIOGroup use after free #14

wants to merge 1 commit into from

Conversation

bingsong
Copy link

The sanitizer reports:

==2534591==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600007f600 at pc 0x557ca6aede56 bp 0x7ffd98571600 sp 0x7ffd985715e0 WRITE of size 8 at 0x60600007f600 thread T0
#0 0x557ca6aede55 in vfio_connect_proxy /home/ctyun/workspace/code/qemu-5.0/hw/vfio/common.c:1936
#1 0x557ca6b32d74 in vfio_user_pci_realize /home/ctyun/workspace/code/qemu-5.0/hw/vfio/pci.c:3649
#2 0x557ca733c425 in pci_qdev_realize hw/pci/pci.c:2098
#3 0x557ca7036370 in device_set_realized hw/core/qdev.c:891
#4 0x557ca7709f27 in property_set_bool qom/object.c:2238
#5 0x557ca771a01e in object_property_set_qobject qom/qom-qobject.c:26
#6 0x557ca77120b8 in object_property_set_bool qom/object.c:1390
#7 0x557ca6f2362e in qdev_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:680
#8 0x557ca6f2419f in qmp_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:805
#9 0x557ca6f24a78 in hmp_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:905
#10 0x557ca7572cdd in handle_hmp_command monitor/hmp.c:1082
#11 0x557ca757323b in monitor_command_cb monitor/hmp.c:47
#12 0x557ca7b0bfc2 in readline_handle_byte util/readline.c:408
#13 0x557ca7573337 in monitor_read monitor/hmp.c:1312
#14 0x557ca7927bee in mux_chr_read chardev/char-mux.c:228
#15 0x557ca7920942 in fd_chr_read chardev/char-fd.c:68
#16 0x7f3f7fa8e183 in g_main_context_dispatch (/usr/lib64/libglib-2.0.so.0+0x51183)
#17 0x557ca7aa7caf in glib_pollfds_poll util/main-loop.c:219
#18 0x557ca7aa7caf in os_host_main_loop_wait util/main-loop.c:242
#19 0x557ca7aa7caf in main_loop_wait util/main-loop.c:518
#20 0x557ca6c7e413 in qemu_main_loop /home/ctyun/workspace/code/qemu-5.0/softmmu/vl.c:1710
#21 0x557ca67570fd in main /home/ctyun/workspace/code/qemu-5.0/softmmu/main.c:49
#22 0x7f3f7e256b16 in __libc_start_main (/usr/lib64/libc.so.6+0x25b16)
#23 0x557ca675a309 in _start (/root/sibs/qemu-system-x86_64+0x18d7309)

0x60600007f600 is located 32 bytes inside of 64-byte region [0x60600007f5e0,0x60600007f620) freed by thread T1 here:
#0 0x7f3f7ff16c89 in free (/usr/lib64/libasan.so.4+0x151c89)
#1 0x7f3f7fa93fa8 in g_free (/usr/lib64/libglib-2.0.so.0+0x56fa8)
#2 0x557ca770f3bf in object_deinit qom/object.c:654
#3 0x557ca770f3bf in object_finalize qom/object.c:668
#4 0x557ca770f3bf in object_unref qom/object.c:1128
#5 0x557ca7e669cf (/root/sibs/qemu-system-x86_64+0x2fe39cf)

previously allocated by thread T0 here:
#0 0x7f3f7ff1735b in calloc (/usr/lib64/libasan.so.4+0x15235b)
#1 0x7f3f7fa93f00 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x56f00)
#2 0x557ca733c425 in pci_qdev_realize hw/pci/pci.c:2098

Thread T1 created by T0 here:
#0 0x7f3f7ff096ac in __interceptor_pthread_create (/usr/lib64/libasan.so.4+0x1446ac)
#1 0x557ca7abaf2f in qemu_thread_create util/qemu-thread-posix.c:556

SUMMARY: AddressSanitizer: heap-use-after-free /home/ctyun/workspace/code/qemu-5.0/hw/vfio/common.c:1936 in vfio_connect_proxy

@oracle-contributor-agreement
Copy link

Thank you for your pull request and welcome to our community! To contribute, please sign the Oracle Contributor Agreement (OCA).
The following contributors of this PR have not signed the OCA:

To sign the OCA, please create an Oracle account and sign the OCA in Oracle's Contributor Agreement Application.

When signing the OCA, please provide your GitHub username. After signing the OCA and getting an OCA approval from Oracle, this PR will be automatically updated.

If you are an Oracle employee, please make sure that you are a member of the main Oracle GitHub organization, and your membership in this organization is public.

@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. label May 22, 2023
@bingsong
vfio/pci: Fix VFIOGroup use after free
d3a005e 
The sanitizer reports:

==2534591==ERROR: AddressSanitizer: heap-use-after-free on address 0x60600007f600 at pc 0x557ca6aede56 bp 0x7ffd98571600 sp 0x7ffd985715e0
WRITE of size 8 at 0x60600007f600 thread T0
    #0 0x557ca6aede55 in vfio_connect_proxy /home/ctyun/workspace/code/qemu-5.0/hw/vfio/common.c:1936
    oracle#1 0x557ca6b32d74 in vfio_user_pci_realize /home/ctyun/workspace/code/qemu-5.0/hw/vfio/pci.c:3649
    oracle#2 0x557ca733c425 in pci_qdev_realize hw/pci/pci.c:2098
    oracle#3 0x557ca7036370 in device_set_realized hw/core/qdev.c:891
    oracle#4 0x557ca7709f27 in property_set_bool qom/object.c:2238
    oracle#5 0x557ca771a01e in object_property_set_qobject qom/qom-qobject.c:26
    oracle#6 0x557ca77120b8 in object_property_set_bool qom/object.c:1390
    oracle#7 0x557ca6f2362e in qdev_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:680
    oracle#8 0x557ca6f2419f in qmp_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:805
    oracle#9 0x557ca6f24a78 in hmp_device_add /home/ctyun/workspace/code/qemu-5.0/qdev-monitor.c:905
    oracle#10 0x557ca7572cdd in handle_hmp_command monitor/hmp.c:1082
    oracle#11 0x557ca757323b in monitor_command_cb monitor/hmp.c:47
    oracle#12 0x557ca7b0bfc2 in readline_handle_byte util/readline.c:408
    oracle#13 0x557ca7573337 in monitor_read monitor/hmp.c:1312
    oracle#14 0x557ca7927bee in mux_chr_read chardev/char-mux.c:228
    oracle#15 0x557ca7920942 in fd_chr_read chardev/char-fd.c:68
    oracle#16 0x7f3f7fa8e183 in g_main_context_dispatch (/usr/lib64/libglib-2.0.so.0+0x51183)
    oracle#17 0x557ca7aa7caf in glib_pollfds_poll util/main-loop.c:219
    oracle#18 0x557ca7aa7caf in os_host_main_loop_wait util/main-loop.c:242
    oracle#19 0x557ca7aa7caf in main_loop_wait util/main-loop.c:518
    oracle#20 0x557ca6c7e413 in qemu_main_loop /home/ctyun/workspace/code/qemu-5.0/softmmu/vl.c:1710
    #21 0x557ca67570fd in main /home/ctyun/workspace/code/qemu-5.0/softmmu/main.c:49
    #22 0x7f3f7e256b16 in __libc_start_main (/usr/lib64/libc.so.6+0x25b16)
    #23 0x557ca675a309 in _start (/root/sibs/qemu-system-x86_64+0x18d7309)

0x60600007f600 is located 32 bytes inside of 64-byte region [0x60600007f5e0,0x60600007f620)
freed by thread T1 here:
    #0 0x7f3f7ff16c89 in free (/usr/lib64/libasan.so.4+0x151c89)
    oracle#1 0x7f3f7fa93fa8 in g_free (/usr/lib64/libglib-2.0.so.0+0x56fa8)
    oracle#2 0x557ca770f3bf in object_deinit qom/object.c:654
    oracle#3 0x557ca770f3bf in object_finalize qom/object.c:668
    oracle#4 0x557ca770f3bf in object_unref qom/object.c:1128
    oracle#5 0x557ca7e669cf  (/root/sibs/qemu-system-x86_64+0x2fe39cf)

previously allocated by thread T0 here:
    #0 0x7f3f7ff1735b in calloc (/usr/lib64/libasan.so.4+0x15235b)
    oracle#1 0x7f3f7fa93f00 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x56f00)
    oracle#2 0x557ca733c425 in pci_qdev_realize hw/pci/pci.c:2098

Thread T1 created by T0 here:
    #0 0x7f3f7ff096ac in __interceptor_pthread_create (/usr/lib64/libasan.so.4+0x1446ac)
    oracle#1 0x557ca7abaf2f in qemu_thread_create util/qemu-thread-posix.c:556

SUMMARY: AddressSanitizer: heap-use-after-free /home/ctyun/workspace/code/qemu-5.0/hw/vfio/common.c:1936 in vfio_connect_proxy

Signed-off-by: Bingsong Si <[email protected]>
@bingsong bingsong force-pushed the fix-use-after-free branch from 59fdfaf to d3a005e Compare May 22, 2023 07:59
@bingsong bingsong closed this May 22, 2023
@bingsong bingsong deleted the fix-use-after-free branch May 22, 2023 08:05
@bingsong bingsong restored the fix-use-after-free branch May 27, 2023 13:51
@bingsong bingsong reopened this May 27, 2023
@oracle-contributor-agreement oracle-contributor-agreement bot added OCA Verified All contributors have signed the Oracle Contributor Agreement. and removed OCA Required At least one contributor does not have an approved Oracle Contributor Agreement. labels May 27, 2023
kheubaum pushed a commit that referenced this pull request Jul 10, 2023
@foxmox @bonzini
qemu_cleanup: begin drained section after vm_shutdown()
ca2a5e6 
in order to avoid requests being stuck in a BlockBackend's request
queue during cleanup. Having such requests can lead to a deadlock [0]
with a virtio-scsi-pci device using iothread that's busy with IO when
initiating a shutdown with QMP 'quit'.

There is a race where such a queued request can continue sometime
(maybe after bdrv_child_free()?) during bdrv_root_unref_child() [1].
The completion will hold the AioContext lock and wait for the BQL
during SCSI completion, but the main thread will hold the BQL and
wait for the AioContext as part of bdrv_root_unref_child(), leading to
the deadlock [0].

[0]:

> Thread 3 (Thread 0x7f3bbd87b700 (LWP 135952) "qemu-system-x86"):
> #0  __lll_lock_wait (futex=futex@entry=0x564183365f00 <qemu_global_mutex>, private=0) at lowlevellock.c:52
> #1  0x00007f3bc1c0d843 in __GI___pthread_mutex_lock (mutex=0x564183365f00 <qemu_global_mutex>) at ../nptl/pthread_mutex_lock.c:80
> #2  0x0000564182939f2e in qemu_mutex_lock_impl (mutex=0x564183365f00 <qemu_global_mutex>, file=0x564182b7f774 "../softmmu/physmem.c", line=2593) at ../util/qemu-thread-posix.c:94
> #3  0x000056418247cc2a in qemu_mutex_lock_iothread_impl (file=0x564182b7f774 "../softmmu/physmem.c", line=2593) at ../softmmu/cpus.c:504
> #4  0x00005641826d5325 in prepare_mmio_access (mr=0x5641856148a0) at ../softmmu/physmem.c:2593
> #5  0x00005641826d6fe7 in address_space_stl_internal (as=0x56418679b310, addr=4276113408, val=16418, attrs=..., result=0x0, endian=DEVICE_LITTLE_ENDIAN) at /home/febner/repos/qemu/memory_ldst.c.inc:318
> #6  0x00005641826d7154 in address_space_stl_le (as=0x56418679b310, addr=4276113408, val=16418, attrs=..., result=0x0) at /home/febner/repos/qemu/memory_ldst.c.inc:357
> #7  0x0000564182374b07 in pci_msi_trigger (dev=0x56418679b0d0, msg=...) at ../hw/pci/pci.c:359
> #8  0x000056418237118b in msi_send_message (dev=0x56418679b0d0, msg=...) at ../hw/pci/msi.c:379
> #9  0x0000564182372c10 in msix_notify (dev=0x56418679b0d0, vector=8) at ../hw/pci/msix.c:542
> #10 0x000056418243719c in virtio_pci_notify (d=0x56418679b0d0, vector=8) at ../hw/virtio/virtio-pci.c:77
> #11 0x00005641826933b0 in virtio_notify_vector (vdev=0x5641867a34a0, vector=8) at ../hw/virtio/virtio.c:1985
> #12 0x00005641826948d6 in virtio_irq (vq=0x5641867ac078) at ../hw/virtio/virtio.c:2461
> #13 0x0000564182694978 in virtio_notify (vdev=0x5641867a34a0, vq=0x5641867ac078) at ../hw/virtio/virtio.c:2473
> #14 0x0000564182665b83 in virtio_scsi_complete_req (req=0x7f3bb000e5d0) at ../hw/scsi/virtio-scsi.c:115
> #15 0x00005641826670ce in virtio_scsi_complete_cmd_req (req=0x7f3bb000e5d0) at ../hw/scsi/virtio-scsi.c:641
> #16 0x000056418266736b in virtio_scsi_command_complete (r=0x7f3bb0010560, resid=0) at ../hw/scsi/virtio-scsi.c:712
> #17 0x000056418239aac6 in scsi_req_complete (req=0x7f3bb0010560, status=2) at ../hw/scsi/scsi-bus.c:1526
> #18 0x000056418239e090 in scsi_handle_rw_error (r=0x7f3bb0010560, ret=-123, acct_failed=false) at ../hw/scsi/scsi-disk.c:242
> #19 0x000056418239e13f in scsi_disk_req_check_error (r=0x7f3bb0010560, ret=-123, acct_failed=false) at ../hw/scsi/scsi-disk.c:265
> #20 0x000056418239e482 in scsi_dma_complete_noio (r=0x7f3bb0010560, ret=-123) at ../hw/scsi/scsi-disk.c:340
> #21 0x000056418239e5d9 in scsi_dma_complete (opaque=0x7f3bb0010560, ret=-123) at ../hw/scsi/scsi-disk.c:371
> #22 0x00005641824809ad in dma_complete (dbs=0x7f3bb000d9d0, ret=-123) at ../softmmu/dma-helpers.c:107
> #23 0x0000564182480a72 in dma_blk_cb (opaque=0x7f3bb000d9d0, ret=-123) at ../softmmu/dma-helpers.c:127
> #24 0x00005641827bf78a in blk_aio_complete (acb=0x7f3bb00021a0) at ../block/block-backend.c:1563
> #25 0x00005641827bfa5e in blk_aio_write_entry (opaque=0x7f3bb00021a0) at ../block/block-backend.c:1630
> #26 0x000056418295638a in coroutine_trampoline (i0=-1342102448, i1=32571) at ../util/coroutine-ucontext.c:177
> #27 0x00007f3bc0caed40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
> #28 0x00007f3bbd8757f0 in ?? ()
> #29 0x0000000000000000 in ?? ()
>
> Thread 1 (Thread 0x7f3bbe3e9280 (LWP 135944) "qemu-system-x86"):
> #0  __lll_lock_wait (futex=futex@entry=0x5641856f2a00, private=0) at lowlevellock.c:52
> #1  0x00007f3bc1c0d8d1 in __GI___pthread_mutex_lock (mutex=0x5641856f2a00) at ../nptl/pthread_mutex_lock.c:115
> #2  0x0000564182939f2e in qemu_mutex_lock_impl (mutex=0x5641856f2a00, file=0x564182c0e319 "../util/async.c", line=728) at ../util/qemu-thread-posix.c:94
> #3  0x000056418293a140 in qemu_rec_mutex_lock_impl (mutex=0x5641856f2a00, file=0x564182c0e319 "../util/async.c", line=728) at ../util/qemu-thread-posix.c:149
> #4  0x00005641829532d5 in aio_context_acquire (ctx=0x5641856f29a0) at ../util/async.c:728
> #5  0x000056418279d5df in bdrv_set_aio_context_commit (opaque=0x5641856e6e50) at ../block.c:7493
> #6  0x000056418294e288 in tran_commit (tran=0x56418630bfe0) at ../util/transactions.c:87
> #7  0x000056418279d880 in bdrv_try_change_aio_context (bs=0x5641856f7130, ctx=0x56418548f810, ignore_child=0x0, errp=0x0) at ../block.c:7626
> #8  0x0000564182793f39 in bdrv_root_unref_child (child=0x5641856f47d0) at ../block.c:3242
> #9  0x00005641827be137 in blk_remove_bs (blk=0x564185709880) at ../block/block-backend.c:914
> #10 0x00005641827bd689 in blk_remove_all_bs () at ../block/block-backend.c:583
> #11 0x0000564182798699 in bdrv_close_all () at ../block.c:5117
> #12 0x000056418248a5b2 in qemu_cleanup () at ../softmmu/runstate.c:821
> #13 0x0000564182738603 in qemu_default_main () at ../softmmu/main.c:38
> #14 0x0000564182738631 in main (argc=30, argv=0x7ffd675a8a48) at ../softmmu/main.c:48
>
> (gdb) p *((QemuMutex*)0x5641856f2a00)
> $1 = {lock = {__data = {__lock = 2, __count = 2, __owner = 135952, ...
> (gdb) p *((QemuMutex*)0x564183365f00)
> $2 = {lock = {__data = {__lock = 2, __count = 0, __owner = 135944, ...

[1]:

> Thread 1 "qemu-system-x86" hit Breakpoint 5, bdrv_drain_all_end () at ../block/io.c:551
> #0  bdrv_drain_all_end () at ../block/io.c:551
> #1  0x00005569810f0376 in bdrv_graph_wrlock (bs=0x0) at ../block/graph-lock.c:156
> #2  0x00005569810bd3e0 in bdrv_replace_child_noperm (child=0x556982e2d7d0, new_bs=0x0) at ../block.c:2897
> #3  0x00005569810bdef2 in bdrv_root_unref_child (child=0x556982e2d7d0) at ../block.c:3227
> #4  0x00005569810e8137 in blk_remove_bs (blk=0x556982e42880) at ../block/block-backend.c:914
> #5  0x00005569810e7689 in blk_remove_all_bs () at ../block/block-backend.c:583
> #6  0x00005569810c2699 in bdrv_close_all () at ../block.c:5117
> #7  0x0000556980db45b2 in qemu_cleanup () at ../softmmu/runstate.c:821
> #8  0x0000556981062603 in qemu_default_main () at ../softmmu/main.c:38
> #9  0x0000556981062631 in main (argc=30, argv=0x7ffd7a82a418) at ../softmmu/main.c:48
> [Switching to Thread 0x7fe76dab2700 (LWP 103649)]
>
> Thread 3 "qemu-system-x86" hit Breakpoint 4, blk_inc_in_flight (blk=0x556982e42880) at ../block/block-backend.c:1505
> #0  blk_inc_in_flight (blk=0x556982e42880) at ../block/block-backend.c:1505
> #1  0x00005569810e8f36 in blk_wait_while_drained (blk=0x556982e42880) at ../block/block-backend.c:1312
> #2  0x00005569810e9231 in blk_co_do_pwritev_part (blk=0x556982e42880, offset=3422961664, bytes=4096, qiov=0x556983028060, qiov_offset=0, flags=0) at ../block/block-backend.c:1402
> #3  0x00005569810e9a4b in blk_aio_write_entry (opaque=0x556982e2cfa0) at ../block/block-backend.c:1628
> #4  0x000055698128038a in coroutine_trampoline (i0=-2090057872, i1=21865) at ../util/coroutine-ucontext.c:177
> #5  0x00007fe770f50d40 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
> #6  0x00007ffd7a829570 in ?? ()
> #7  0x0000000000000000 in ?? ()

Signed-off-by: Fiona Ebner <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
kheubaum pushed a commit that referenced this pull request Sep 12, 2023
@kevmw
virtio: Drop out of coroutine context in virtio_load()
92e2e6a 
virtio_load() as a whole should run in coroutine context because it
reads from the migration stream and we don't want this to block.

However, it calls virtio_set_features_nocheck() and devices don't
expect their .set_features callback to run in a coroutine and therefore
call functions that may not be called in coroutine context. To fix this,
drop out of coroutine context for calling virtio_set_features_nocheck().

Without this fix, the following crash was reported:

  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
  #1  0x00007efc738c05d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
  #2  0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007efc738477f3 in __GI_abort () at abort.c:79
  #4  0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()",
     file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", line=line@entry=275, function=function@entry=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:92
  #5  0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275,
     function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:101
  #6  0x0000560aebcd8dd6 in bdrv_register_buf ()
  #7  0x0000560aeb97ed97 in ram_block_added.llvm ()
  #8  0x0000560aebb8303f in ram_block_add.llvm ()
  #9  0x0000560aebb834fa in qemu_ram_alloc_internal.llvm ()
  #10 0x0000560aebb2ac98 in vfio_region_mmap ()
  #11 0x0000560aebb3ea0f in vfio_bars_register ()
  #12 0x0000560aebb3c628 in vfio_realize ()
  #13 0x0000560aeb90f0c2 in pci_qdev_realize ()
  #14 0x0000560aebc40305 in device_set_realized ()
  #15 0x0000560aebc48e07 in property_set_bool.llvm ()
  #16 0x0000560aebc46582 in object_property_set ()
  #17 0x0000560aebc4cd58 in object_property_set_qobject ()
  #18 0x0000560aebc46ba7 in object_property_set_bool ()
  #19 0x0000560aeb98b3ca in qdev_device_add_from_qdict ()
  #20 0x0000560aebb1fbaf in virtio_net_set_features ()
  #21 0x0000560aebb46b51 in virtio_set_features_nocheck ()
  #22 0x0000560aebb47107 in virtio_load ()
  #23 0x0000560aeb9ae7ce in vmstate_load_state ()
  #24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main ()
  #25 0x0000560aeb9d45e1 in qemu_loadvm_state ()
  #26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm ()
  #27 0x0000560aebeace56 in coroutine_trampoline.llvm ()

Cc: [email protected]
Buglink: https://issues.redhat.com/browse/RHEL-832
Signed-off-by: Kevin Wolf <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
kheubaum pushed a commit that referenced this pull request Feb 8, 2024
@kevmw
virtio: Drop out of coroutine context in virtio_load()
df33ce9 
virtio_load() as a whole should run in coroutine context because it
reads from the migration stream and we don't want this to block.

However, it calls virtio_set_features_nocheck() and devices don't
expect their .set_features callback to run in a coroutine and therefore
call functions that may not be called in coroutine context. To fix this,
drop out of coroutine context for calling virtio_set_features_nocheck().

Without this fix, the following crash was reported:

  #0  __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at pthread_kill.c:44
  #1  0x00007efc738c05d3 in __pthread_kill_internal (signo=6, threadid=<optimized out>) at pthread_kill.c:78
  #2  0x00007efc73873d26 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26
  #3  0x00007efc738477f3 in __GI_abort () at abort.c:79
  #4  0x00007efc7384771b in __assert_fail_base (fmt=0x7efc739dbcb8 "", assertion=assertion@entry=0x560aebfbf5cf "!qemu_in_coroutine()",
     file=file@entry=0x560aebfcd2d4 "../block/graph-lock.c", line=line@entry=275, function=function@entry=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:92
  #5  0x00007efc7386ccc6 in __assert_fail (assertion=0x560aebfbf5cf "!qemu_in_coroutine()", file=0x560aebfcd2d4 "../block/graph-lock.c", line=275,
     function=0x560aebfcd34d "void bdrv_graph_rdlock_main_loop(void)") at assert.c:101
  #6  0x0000560aebcd8dd6 in bdrv_register_buf ()
  #7  0x0000560aeb97ed97 in ram_block_added.llvm ()
  #8  0x0000560aebb8303f in ram_block_add.llvm ()
  #9  0x0000560aebb834fa in qemu_ram_alloc_internal.llvm ()
  #10 0x0000560aebb2ac98 in vfio_region_mmap ()
  #11 0x0000560aebb3ea0f in vfio_bars_register ()
  #12 0x0000560aebb3c628 in vfio_realize ()
  #13 0x0000560aeb90f0c2 in pci_qdev_realize ()
  #14 0x0000560aebc40305 in device_set_realized ()
  #15 0x0000560aebc48e07 in property_set_bool.llvm ()
  #16 0x0000560aebc46582 in object_property_set ()
  #17 0x0000560aebc4cd58 in object_property_set_qobject ()
  #18 0x0000560aebc46ba7 in object_property_set_bool ()
  #19 0x0000560aeb98b3ca in qdev_device_add_from_qdict ()
  #20 0x0000560aebb1fbaf in virtio_net_set_features ()
  #21 0x0000560aebb46b51 in virtio_set_features_nocheck ()
  #22 0x0000560aebb47107 in virtio_load ()
  #23 0x0000560aeb9ae7ce in vmstate_load_state ()
  #24 0x0000560aeb9d2ee9 in qemu_loadvm_state_main ()
  #25 0x0000560aeb9d45e1 in qemu_loadvm_state ()
  #26 0x0000560aeb9bc32c in process_incoming_migration_co.llvm ()
  #27 0x0000560aebeace56 in coroutine_trampoline.llvm ()

Cc: [email protected]
Buglink: https://issues.redhat.com/browse/RHEL-832
Signed-off-by: Kevin Wolf <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Stefan Hajnoczi <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
(cherry picked from commit 92e2e6a)
Signed-off-by: Michael Tokarev <[email protected]>
jlevon pushed a commit to jlevon/qemu that referenced this pull request Dec 5, 2024
@foxmox
block/reqlist: allow adding overlapping requests
6475155 
Allow overlapping request by removing the assert that made it
impossible. There are only two callers:

1. block_copy_task_create()

It already asserts the very same condition before calling
reqlist_init_req().

2. cbw_snapshot_read_lock()

There is no need to have read requests be non-overlapping in
copy-before-write when used for snapshot-access. In fact, there was no
protection against two callers of cbw_snapshot_read_lock() calling
reqlist_init_req() with overlapping ranges and this could lead to an
assertion failure [1].

In particular, with the reproducer script below [0], two
cbw_co_snapshot_block_status() callers could race, with the second
calling reqlist_init_req() before the first one finishes and removes
its conflicting request.

[0]:

> #!/bin/bash -e
> dd if=/dev/urandom of=/tmp/disk.raw bs=1M count=1024
> ./qemu-img create /tmp/fleecing.raw -f raw 1G
> (
> ./qemu-system-x86_64 --qmp stdio \
> --blockdev raw,node-name=node0,file.driver=file,file.filename=/tmp/disk.raw \
> --blockdev raw,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.raw \
> <<EOF
> {"execute": "qmp_capabilities"}
> {"execute": "blockdev-add", "arguments": { "driver": "copy-before-write", "file": "node0", "target": "node1", "node-name": "node3" } }
> {"execute": "blockdev-add", "arguments": { "driver": "snapshot-access", "file": "node3", "node-name": "snap0" } }
> {"execute": "nbd-server-start", "arguments": {"addr": { "type": "unix", "data": { "path": "/tmp/nbd.socket" } } } }
> {"execute": "block-export-add", "arguments": {"id": "exp0", "node-name": "snap0", "type": "nbd", "name": "exp0"}}
> EOF
> ) &
> sleep 5
> while true; do
> ./qemu-nbd -d /dev/nbd0
> ./qemu-nbd -c /dev/nbd0 nbd:unix:/tmp/nbd.socket:exportname=exp0 -f raw -r
> nbdinfo --map 'nbd+unix:///exp0?socket=/tmp/nbd.socket'
> done

[1]:

> oracle#5  0x000071e5f0088eb2 in __GI___assert_fail (...) at ./assert/assert.c:101
> oracle#6  0x0000615285438017 in reqlist_init_req (...) at ../block/reqlist.c:23
> oracle#7  0x00006152853e2d98 in cbw_snapshot_read_lock (...) at ../block/copy-before-write.c:237
> oracle#8  0x00006152853e3068 in cbw_co_snapshot_block_status (...) at ../block/copy-before-write.c:304
> oracle#9  0x00006152853f4d22 in bdrv_co_snapshot_block_status (...) at ../block/io.c:3726
> oracle#10 0x000061528543a63e in snapshot_access_co_block_status (...) at ../block/snapshot-access.c:48
> oracle#11 0x00006152853f1a0a in bdrv_co_do_block_status (...) at ../block/io.c:2474
> oracle#12 0x00006152853f2016 in bdrv_co_common_block_status_above (...) at ../block/io.c:2652
> oracle#13 0x00006152853f22cf in bdrv_co_block_status_above (...) at ../block/io.c:2732
> oracle#14 0x00006152853d9a86 in blk_co_block_status_above (...) at ../block/block-backend.c:1473
> oracle#15 0x000061528538da6c in blockstatus_to_extents (...) at ../nbd/server.c:2374
> oracle#16 0x000061528538deb1 in nbd_co_send_block_status (...) at ../nbd/server.c:2481
> oracle#17 0x000061528538f424 in nbd_handle_request (...) at ../nbd/server.c:2978
> oracle#18 0x000061528538f906 in nbd_trip (...) at ../nbd/server.c:3121
> oracle#19 0x00006152855a7caf in coroutine_trampoline (...) at ../util/coroutine-ucontext.c:175

Cc: [email protected]
Suggested-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Fiona Ebner <[email protected]>
Message-Id: <[email protected]>
Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]>
jlevon pushed a commit to jlevon/qemu that referenced this pull request Dec 5, 2024
@snimmagadda @kostyanf14
qemu-ga: Fix a SIGSEGV in ga_run_command() helper
9cfe110 
qemu-ga on a NetBSD -current VM terminates with a SIGSEGV upon receiving
'guest-set-time' command...

Core was generated by `qemu-ga'.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
88	        *str[len] = '\0';
[Current thread is 1 (process 1112)]
(gdb) bt
#0  0x000000000cd37a40 in ga_pipe_read_str (fd=fd@entry=0xffffff922a20, str=str@entry=0xffffff922a18)
    at ../qga/commands-posix.c:88
oracle#1  0x000000000cd37b60 in ga_run_command (argv=argv@entry=0xffffff922a90,
    action=action@entry=0xcda34b8 "set hardware clock to system time", errp=errp@entry=0xffffff922a70, in_str=0x0)
    at ../qga/commands-posix.c:164
oracle#2  0x000000000cd380c4 in qmp_guest_set_time (has_time=<optimized out>, time_ns=<optimized out>,
    errp=errp@entry=0xffffff922ad0) at ../qga/commands-posix.c:304
oracle#3  0x000000000cd253d8 in qmp_marshal_guest_set_time (args=<optimized out>, ret=<optimized out>, errp=0xffffff922b48)
    at qga/qga-qapi-commands.c:193
oracle#4  0x000000000cd4e71c in qmp_dispatch (cmds=cmds@entry=0xcdf5b18 <ga_commands>, request=request@entry=0xf3c711a4b000,
    allow_oob=allow_oob@entry=false, cur_mon=cur_mon@entry=0x0) at ../qapi/qmp-dispatch.c:220
oracle#5  0x000000000cd36524 in process_event (opaque=0xf3c711a79000, obj=0xf3c711a4b000, err=0x0) at ../qga/main.c:677
oracle#6  0x000000000cd526f0 in json_message_process_token (lexer=lexer@entry=0xf3c711a79018, input=0xf3c712072480,
    type=type@entry=JSON_RCURLY, x=28, y=1) at ../qobject/json-streamer.c:99
oracle#7  0x000000000cd93860 in json_lexer_feed_char (lexer=lexer@entry=0xf3c711a79018, ch=125 '}', flush=flush@entry=false)
    at ../qobject/json-lexer.c:313
oracle#8  0x000000000cd93a00 in json_lexer_feed (lexer=lexer@entry=0xf3c711a79018,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-lexer.c:350
oracle#9  0x000000000cd5290c in json_message_parser_feed (parser=parser@entry=0xf3c711a79000,
    buffer=buffer@entry=0xffffff922d10 "{\"execute\":\"guest-set-time\"}\n", size=<optimized out>)
    at ../qobject/json-streamer.c:121
oracle#10 0x000000000cd361fc in channel_event_cb (condition=<optimized out>, data=0xf3c711a79000) at ../qga/main.c:703
oracle#11 0x000000000cd3710c in ga_channel_client_event (channel=<optimized out>, condition=<optimized out>, data=0xf3c711b2d300)
    at ../qga/channel-posix.c:94
oracle#12 0x0000f3c7120d9bec in g_main_dispatch () from /usr/pkg/lib/libglib-2.0.so.0
oracle#13 0x0000f3c7120dd25c in g_main_context_iterate_unlocked.constprop () from /usr/pkg/lib/libglib-2.0.so.0
oracle#14 0x0000f3c7120ddbf0 in g_main_loop_run () from /usr/pkg/lib/libglib-2.0.so.0
oracle#15 0x000000000cda00d8 in run_agent_once (s=0xf3c711a79000) at ../qga/main.c:1522
oracle#16 run_agent (s=0xf3c711a79000) at ../qga/main.c:1559
oracle#17 main (argc=<optimized out>, argv=<optimized out>) at ../qga/main.c:1671
(gdb)

The commandline options used on the host machine...
qemu-system-aarch64 \
   -machine type=virt,pflash0=rom \
   -m 8G \
   -cpu host \
   -smp 8 \
   -accel hvf \
   -device virtio-net-pci,netdev=unet \
   -device virtio-blk-pci,drive=hd \
   -drive file=netbsd.qcow2,if=none,id=hd \
   -netdev user,id=unet,hostfwd=tcp::2223-:22 \
   -object rng-random,filename=/dev/urandom,id=viornd0 \
   -device virtio-rng-pci,rng=viornd0 \
   -serial mon:stdio \
   -display none \
   -blockdev node-name=rom,driver=file,filename=/opt/homebrew/Cellar/qemu/9.0.2/share/qemu/edk2-aarch64-code.fd,read-only=true \
   -chardev socket,path=/tmp/qga_netbsd.sock,server=on,wait=off,id=qga0 \
   -device virtio-serial \
   -device virtconsole,chardev=qga0,name=org.qemu.guest_agent.0

This patch rectifies the operator precedence while assigning the NUL
terminator.

Fixes: c3f32c1

Signed-off-by: Sunil Nimmagadda <[email protected]>
Reviewed-by: Konstantin Kostiuk <[email protected]>
Reviewed-by: Daniel P. Berrangé <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Konstantin Kostiuk <[email protected]>
jlevon pushed a commit to jlevon/qemu that referenced this pull request Jan 7, 2025
@pbo-linaro
vvfat: fix ubsan issue in create_long_filename
0cb3ff7 
Found with test sbsaref introduced in [1].

[1] https://patchew.org/QEMU/[email protected]/

../block/vvfat.c:433:24: runtime error: index 14 out of bounds for type 'uint8_t [11]'
    #0 0x56151a66b93a in create_long_filename ../block/vvfat.c:433
    oracle#1 0x56151a66f3d7 in create_short_and_long_name ../block/vvfat.c:725
    oracle#2 0x56151a670403 in read_directory ../block/vvfat.c:804
    oracle#3 0x56151a674432 in init_directories ../block/vvfat.c:964
    oracle#4 0x56151a67867b in vvfat_open ../block/vvfat.c:1258
    oracle#5 0x56151a3b8e19 in bdrv_open_driver ../block.c:1660
    oracle#6 0x56151a3bb666 in bdrv_open_common ../block.c:1985
    oracle#7 0x56151a3cadb9 in bdrv_open_inherit ../block.c:4153
    oracle#8 0x56151a3c8850 in bdrv_open_child_bs ../block.c:3731
    oracle#9 0x56151a3ca832 in bdrv_open_inherit ../block.c:4098
    oracle#10 0x56151a3cbe40 in bdrv_open ../block.c:4248
    oracle#11 0x56151a46344f in blk_new_open ../block/block-backend.c:457
    oracle#12 0x56151a388bd9 in blockdev_init ../blockdev.c:612
    oracle#13 0x56151a38ab2d in drive_new ../blockdev.c:1006
    oracle#14 0x5615190fca41 in drive_init_func ../system/vl.c:649
    oracle#15 0x56151aa796dd in qemu_opts_foreach ../util/qemu-option.c:1135
    oracle#16 0x5615190fd2b6 in configure_blockdev ../system/vl.c:708
    oracle#17 0x56151910a307 in qemu_create_early_backends ../system/vl.c:2004
    oracle#18 0x561519113fcf in qemu_init ../system/vl.c:3685
    oracle#19 0x56151a7e438e in main ../system/main.c:47
    oracle#20 0x7f72d1a46249 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #21 0x7f72d1a46304 in __libc_start_main_impl ../csu/libc-start.c:360
    #22 0x561517e98510 in _start (/home/user/.work/qemu/build/qemu-system-aarch64+0x3b9b510)

The offset used can easily go beyond entry->name size. It's probably a
bug, but I don't have the time to dive into vfat specifics for now.

This change solves the ubsan issue, and is functionally equivalent, as
anything written past the entry->name array would not be read anyway.

Signed-off-by: Pierrick Bouvier <[email protected]>
Reviewed-by: Michael Tokarev <[email protected]>
Signed-off-by: Michael Tokarev <[email protected]>
jlevon pushed a commit to jlevon/qemu that referenced this pull request Feb 18, 2025
@kevmw
block: Fix leak in send_qmp_error_event
23ea425 
ASAN detected a leak when running the ahci-test
/ahci/io/dma/lba28/retry:

Direct leak of 35 byte(s) in 1 object(s) allocated from:
    #0 in malloc
    oracle#1 in __vasprintf_internal
    oracle#2 in vasprintf
    oracle#3 in g_vasprintf
    oracle#4 in g_strdup_vprintf
    oracle#5 in g_strdup_printf
    oracle#6 in object_get_canonical_path ../qom/object.c:2096:19
    oracle#7 in blk_get_attached_dev_id_or_path ../block/block-backend.c:1033:12
    oracle#8 in blk_get_attached_dev_path ../block/block-backend.c:1047:12
    oracle#9 in send_qmp_error_event ../block/block-backend.c:2140:36
    oracle#10 in blk_error_action ../block/block-backend.c:2172:9
    oracle#11 in ide_handle_rw_error ../hw/ide/core.c:875:5
    oracle#12 in ide_dma_cb ../hw/ide/core.c:894:13
    oracle#13 in dma_complete ../system/dma-helpers.c:107:9
    oracle#14 in dma_blk_cb ../system/dma-helpers.c:129:9
    oracle#15 in blk_aio_complete ../block/block-backend.c:1552:9
    oracle#16 in blk_aio_write_entry ../block/block-backend.c:1619:5
    oracle#17 in coroutine_trampoline ../util/coroutine-ucontext.c:175:9

Plug the leak by freeing the device path string.

Signed-off-by: Fabiano Rosas <[email protected]>
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
[PMD: Use g_autofree]
Signed-off-by: Philippe Mathieu-Daudé <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
jlevon pushed a commit to jlevon/qemu that referenced this pull request Feb 18, 2025
@xzpeter @kevmw
scripts/qemu-gdb: Support coroutine dumps in coredumps
772f868 
Dumping coroutines don't yet work with coredumps.  Let's make it work.

We still kept most of the old code because they can be either more
flexible, or prettier.  Only add the fallbacks when they stop working.

Currently the raw unwind is pretty ugly, but it works, like this:

  (gdb) qemu bt
  #0  process_incoming_migration_co (opaque=0x0) at ../migration/migration.c:788
  oracle#1  0x000055ae6c0dc4d9 in coroutine_trampoline (i0=-1711718576, i1=21934) at ../util/coroutine-ucontext.c:175
  oracle#2  0x00007f9f59d72f40 in ??? () at /lib64/libc.so.6
  oracle#3  0x00007ffd549214a0 in ??? ()
  oracle#4  0x0000000000000000 in ??? ()
  Coroutine at 0x7f9f4c57c748:
  #0  0x55ae6c0dc9a8 in qemu_coroutine_switch<+120> () at ../util/coroutine-ucontext.c:321
  oracle#1  0x55ae6c0da2f8 in qemu_aio_coroutine_enter<+356> () at ../util/qemu-coroutine.c:293
  oracle#2  0x55ae6c0da3f1 in qemu_coroutine_enter<+34> () at ../util/qemu-coroutine.c:316
  oracle#3  0x55ae6baf775e in migration_incoming_process<+43> () at ../migration/migration.c:876
  oracle#4  0x55ae6baf7ab4 in migration_ioc_process_incoming<+490> () at ../migration/migration.c:1008
  oracle#5  0x55ae6bae9ae7 in migration_channel_process_incoming<+145> () at ../migration/channel.c:45
  oracle#6  0x55ae6bb18e35 in socket_accept_incoming_migration<+118> () at ../migration/socket.c:132
  oracle#7  0x55ae6be939ef in qio_net_listener_channel_func<+131> () at ../io/net-listener.c:54
  oracle#8  0x55ae6be8ce1a in qio_channel_fd_source_dispatch<+78> () at ../io/channel-watch.c:84
  oracle#9  0x7f9f5b26728c in g_main_context_dispatch_unlocked.lto_priv<+315> ()
  oracle#10  0x7f9f5b267555 in g_main_context_dispatch<+36> ()
  oracle#11  0x55ae6c0d91a7 in glib_pollfds_poll<+90> () at ../util/main-loop.c:287
  oracle#12  0x55ae6c0d9235 in os_host_main_loop_wait<+128> () at ../util/main-loop.c:310
  oracle#13  0x55ae6c0d9364 in main_loop_wait<+203> () at ../util/main-loop.c:589
  oracle#14  0x55ae6bac212a in qemu_main_loop<+41> () at ../system/runstate.c:835
  oracle#15  0x55ae6bfdf522 in qemu_default_main<+19> () at ../system/main.c:37
  oracle#16  0x55ae6bfdf55f in main<+40> () at ../system/main.c:48
  oracle#17  0x7f9f59d42248 in __libc_start_call_main<+119> ()
  oracle#18  0x7f9f59d4230b in __libc_start_main_impl<+138> ()

Signed-off-by: Peter Xu <[email protected]>
Message-ID: <[email protected]>
Reviewed-by: Kevin Wolf <[email protected]>
Signed-off-by: Kevin Wolf <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@bingsong