-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
Support ARConnect (MCIP) interface #2
Labels
Comments
Open
abrodkin
added a commit
that referenced
this issue
Feb 6, 2021
AUX registers are used in ARC processors to deal with settings or internal states of different internals like built-in timers, interrupt controller(s), caches etc. Though for us here interrupts and timers are of the main interest as those are very good examples of IO operations and we do need explicitly allow it to make "icount" subsystem happy, as otherwise on the first attempt to set ARC built-in timer LIMIT register we see icount_get_raw_locked() barking: "qemu-system-arc: Bad icount read", and that's because: ------------------>8----------------- | gdb --args ./build/qemu-system-arc ... -icount auto | | ... | | (gdb) b icount.c:116 | Breakpoint 1 at 0x54178a: file ../softmmu/icount.c, line 116. | | (gdb) r | | Thread 3 "qemu-system-arc" hit Breakpoint 1, icount_get_raw_locked () at ../softmmu/icount.c:116 | 116 error_report("Bad icount read"); | (gdb) bt | #0 icount_get_raw_locked () at ../softmmu/icount.c:116 | #1 0x0000555555a957d5 in icount_get_locked () at ../softmmu/icount.c:128 | #2 0x0000555555a9586d in icount_get () at ../softmmu/icount.c:154 | #3 0x0000555555a901fd in tcg_get_virtual_clock () at ../accel/tcg/tcg-cpus.c:524 | #4 0x0000555555a0fe05 in cpus_get_virtual_clock () at ../softmmu/cpus.c:211 | #5 0x0000555555c88fd4 in qemu_clock_get_ns (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:638 | #6 0x00005555559aa8eb in cpu_arc_timer_update (env=0x5555565280a0, timer=0) at ../target/arc/timer.c:42 | #7 0x00005555559ab272 in cpu_arc_store_limit (env=0x5555565280a0, timer=0, value=500000) at ../target/arc/timer.c:246 | #8 0x00005555559aba04 in aux_timer_set (aux_reg_detail=0x5555562eedb0 <arc_aux_regs_detail+2352>, val=500000, data=0x5555565280a0) at ../target/arc/timer.c:436 | #9 0x00005555559a4cb2 in helper_sr (env=0x5555565280a0, val=500000, aux=35) at ../target/arc/op_helper.c:209 | #10 0x00007fffb041dc6a in code_gen_buffer () | #11 0x0000555555a2f6c8 in cpu_tb_exec (cpu=0x55555651f960, itb=0x7fffb041db00 <code_gen_buffer+4315859>) at ../accel/tcg/cpu-exec.c:178 | #12 0x0000555555a304ae in cpu_loop_exec_tb (cpu=0x55555651f960, tb=0x7fffb041db00 <code_gen_buffer+4315859>, last_tb=0x7ffff6013928, tb_exit=0x7ffff6013920) | at ../accel/tcg/cpu-exec.c:658 | #13 0x0000555555a307a6 in cpu_exec (cpu=0x55555651f960) at ../accel/tcg/cpu-exec.c:771 | #14 0x0000555555a8f911 in tcg_cpu_exec (cpu=0x55555651f960) at ../accel/tcg/tcg-cpus.c:243 | #15 0x0000555555a8fc12 in tcg_rr_cpu_thread_fn (arg=0x55555651f960) at ../accel/tcg/tcg-cpus.c:346 | #16 0x0000555555c9562e in qemu_thread_start (args=0x5555565339e0) at ../util/qemu-thread-posix.c:521 | #17 0x00007ffff7899609 in start_thread (arg=<optimized out>) at pthread_create.c:477 | #18 0x00007ffff77c0293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ------------------>8----------------- Fix that, hinting QEMU about possible IO on access of AUX regs, which are only possible via LR & SR instructions. Signed-off-by: Alexey Brodkin <[email protected]>
abrodkin
added a commit
that referenced
this issue
Feb 6, 2021
AUX registers are used in ARC processors to deal with settings or internal states of different internals like built-in timers, interrupt controller(s), caches etc. Though for us here interrupts and timers are of the main interest as those are very good examples of IO operations and we do need explicitly allow it to make "icount" subsystem happy, as otherwise on the first attempt to set ARC built-in timer LIMIT register we see icount_get_raw_locked() barking: "qemu-system-arc: Bad icount read", and that's because: ------------------>8----------------- | gdb --args ./build/qemu-system-arc ... -icount auto | | ... | | (gdb) b icount.c:116 | Breakpoint 1 at 0x54178a: file ../softmmu/icount.c, line 116. | | (gdb) r | | Thread 3 "qemu-system-arc" hit Breakpoint 1, icount_get_raw_locked () at ../softmmu/icount.c:116 | 116 error_report("Bad icount read"); | (gdb) bt | #0 icount_get_raw_locked () at ../softmmu/icount.c:116 | #1 0x0000555555a957d5 in icount_get_locked () at ../softmmu/icount.c:128 | #2 0x0000555555a9586d in icount_get () at ../softmmu/icount.c:154 | #3 0x0000555555a901fd in tcg_get_virtual_clock () at ../accel/tcg/tcg-cpus.c:524 | #4 0x0000555555a0fe05 in cpus_get_virtual_clock () at ../softmmu/cpus.c:211 | #5 0x0000555555c88fd4 in qemu_clock_get_ns (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:638 | #6 0x00005555559aa8eb in cpu_arc_timer_update (env=0x5555565280a0, timer=0) at ../target/arc/timer.c:42 | #7 0x00005555559ab272 in cpu_arc_store_limit (env=0x5555565280a0, timer=0, value=500000) at ../target/arc/timer.c:246 | #8 0x00005555559aba04 in aux_timer_set (aux_reg_detail=0x5555562eedb0 <arc_aux_regs_detail+2352>, val=500000, data=0x5555565280a0) at ../target/arc/timer.c:436 | #9 0x00005555559a4cb2 in helper_sr (env=0x5555565280a0, val=500000, aux=35) at ../target/arc/op_helper.c:209 | #10 0x00007fffb041dc6a in code_gen_buffer () | #11 0x0000555555a2f6c8 in cpu_tb_exec (cpu=0x55555651f960, itb=0x7fffb041db00 <code_gen_buffer+4315859>) at ../accel/tcg/cpu-exec.c:178 | #12 0x0000555555a304ae in cpu_loop_exec_tb (cpu=0x55555651f960, tb=0x7fffb041db00 <code_gen_buffer+4315859>, last_tb=0x7ffff6013928, tb_exit=0x7ffff6013920) | at ../accel/tcg/cpu-exec.c:658 | #13 0x0000555555a307a6 in cpu_exec (cpu=0x55555651f960) at ../accel/tcg/cpu-exec.c:771 | #14 0x0000555555a8f911 in tcg_cpu_exec (cpu=0x55555651f960) at ../accel/tcg/tcg-cpus.c:243 | #15 0x0000555555a8fc12 in tcg_rr_cpu_thread_fn (arg=0x55555651f960) at ../accel/tcg/tcg-cpus.c:346 | #16 0x0000555555c9562e in qemu_thread_start (args=0x5555565339e0) at ../util/qemu-thread-posix.c:521 | #17 0x00007ffff7899609 in start_thread (arg=<optimized out>) at pthread_create.c:477 | #18 0x00007ffff77c0293 in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 ------------------>8----------------- Fix that, hinting QEMU about possible IO on access of AUX regs, which are only possible via LR & SR instructions. Signed-off-by: Alexey Brodkin <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Apr 5, 2021
Commit v5.2.0-190-g0546c0609c ("vl: split various early command line options to a separate function") moved the trace backend init code to the qemu_process_early_options(). Which is now being called before os_daemonize() via qemu_maybe_daemonize(). Turns out that this change of order causes a problem when executing QEMU in daemon mode and with CONFIG_TRACE_SIMPLE. The trace thread is now being created by the parent, and the parent is left waiting for a trace file flush that was registered via st_init(). The result is that the parent process never exits. To reproduce, fire up a QEMU process with -daemonize and with CONFIG_TRACE_SIMPLE enabled. Two QEMU process will be left in the host: $ sudo ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults \ -nographic -machine none,accel=kvm:tcg -daemonize $ ps axf | grep qemu 529710 pts/3 S+ 0:00 | \_ grep --color=auto qemu 529697 ? Ssl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize 529699 ? Sl 0:00 \_ ./x86_64-softmmu/qemu-system-x86_64 -S -no-user-config -nodefaults -nographic -machine none,accel=kvm:tcg -daemonize The parent thread is hang in flush_trace_file: $ sudo gdb ./x86_64-softmmu/qemu-system-x86_64 529697 (..) (gdb) bt #0 0x00007f9dac6a137d in syscall () at /lib64/libc.so.6 #1 0x00007f9dacc3c4f3 in g_cond_wait () at /lib64/libglib-2.0.so.0 #2 0x0000555d12f952da in flush_trace_file (wait=true) at ../trace/simple.c:140 #3 0x0000555d12f95b4c in st_flush_trace_buffer () at ../trace/simple.c:383 #4 0x00007f9dac5e43a7 in __run_exit_handlers () at /lib64/libc.so.6 #5 0x00007f9dac5e4550 in on_exit () at /lib64/libc.so.6 #6 0x0000555d12d454de in os_daemonize () at ../os-posix.c:255 #7 0x0000555d12d0bd5c in qemu_maybe_daemonize (pid_file=0x0) at ../softmmu/vl.c:2408 #8 0x0000555d12d0e566 in qemu_init (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/vl.c:3459 #9 0x0000555d128edac1 in main (argc=8, argv=0x7fffc594d9b8, envp=0x7fffc594da00) at ../softmmu/main.c:49 (gdb) Aside from the 'zombie' process in the host, this is directly impacting Libvirt. Libvirt waits for the parent process to exit to be sure that the QMP monitor is available in the daemonized process to fetch QEMU capabilities, and as is now Libvirt hangs at daemon start waiting for the parent thread to exit. The fix is simple: just move the trace backend related code back to be executed after daemonizing. Fixes: 0546c06 Reviewed-by: Paolo Bonzini <[email protected]> Signed-off-by: Daniel Henrique Barboza <[email protected]> Message-Id: <[email protected]> Acked-by: Stefan Hajnoczi <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Apr 5, 2021
We can't know the caller read enough data in the memory pointed by ext_hdr to cast it as a ip6_ext_hdr_routing. Declare rt_hdr on the stack and fill it again from the iovec. Since we already checked there is enough data in the iovec buffer, simply add an assert() call to consume the bytes_read variable. This fix a 2 bytes buffer overrun in eth_parse_ipv6_hdr() reported by QEMU fuzzer: $ cat << EOF | ./qemu-system-i386 -M pc-q35-5.0 \ -accel qtest -monitor none \ -serial none -nographic -qtest stdio outl 0xcf8 0x80001010 outl 0xcfc 0xe1020000 outl 0xcf8 0x80001004 outw 0xcfc 0x7 write 0x25 0x1 0x86 write 0x26 0x1 0xdd write 0x4f 0x1 0x2b write 0xe1020030 0x4 0x190002e1 write 0xe102003a 0x2 0x0807 write 0xe1020048 0x4 0x12077cdd write 0xe1020400 0x4 0xba077cdd write 0xe1020420 0x4 0x190002e1 write 0xe1020428 0x4 0x3509d807 write 0xe1020438 0x1 0xe2 EOF ================================================================= ==2859770==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffdef904902 at pc 0x561ceefa78de bp 0x7ffdef904820 sp 0x7ffdef904818 READ of size 1 at 0x7ffdef904902 thread T0 #0 0x561ceefa78dd in _eth_get_rss_ex_dst_addr net/eth.c:410:17 #1 0x561ceefa41fb in eth_parse_ipv6_hdr net/eth.c:532:17 #2 0x561cef7de639 in net_tx_pkt_parse_headers hw/net/net_tx_pkt.c:228:14 #3 0x561cef7dbef4 in net_tx_pkt_parse hw/net/net_tx_pkt.c:273:9 #4 0x561ceec29f22 in e1000e_process_tx_desc hw/net/e1000e_core.c:730:29 #5 0x561ceec28eac in e1000e_start_xmit hw/net/e1000e_core.c:927:9 #6 0x561ceec1baab in e1000e_set_tdt hw/net/e1000e_core.c:2444:9 #7 0x561ceebf300e in e1000e_core_write hw/net/e1000e_core.c:3256:9 #8 0x561cef3cd4cd in e1000e_mmio_write hw/net/e1000e.c:110:5 Address 0x7ffdef904902 is located in stack of thread T0 at offset 34 in frame #0 0x561ceefa320f in eth_parse_ipv6_hdr net/eth.c:486 This frame has 1 object(s): [32, 34) 'ext_hdr' (line 487) <== Memory access at offset 34 overflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism, swapcontext or vfork (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow net/eth.c:410:17 in _eth_get_rss_ex_dst_addr Shadow bytes around the buggy address: 0x10003df188d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df188e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df188f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18900: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18910: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 =>0x10003df18920:[02]f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18930: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18940: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18950: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18960: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10003df18970: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Shadow byte legend (one shadow byte represents 8 application bytes): Addressable: 00 Partially addressable: 01 02 03 04 05 06 07 Stack left redzone: f1 Stack right redzone: f3 ==2859770==ABORTING Add the corresponding qtest case with the fuzzer reproducer. FWIW GCC 11 similarly reported: net/eth.c: In function 'eth_parse_ipv6_hdr': net/eth.c:410:15: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds] 410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) { | ~~~~~^~~~~~~ net/eth.c:485:24: note: while referencing 'ext_hdr' 485 | struct ip6_ext_hdr ext_hdr; | ^~~~~~~ net/eth.c:410:38: error: array subscript 'struct ip6_ext_hdr_routing[0]' is partly outside array bounds of 'struct ip6_ext_hdr[1]' [-Werror=array-bounds] 410 | if ((rthdr->rtype == 2) && (rthdr->segleft == 1)) { | ~~~~~^~~~~~~~~ net/eth.c:485:24: note: while referencing 'ext_hdr' 485 | struct ip6_ext_hdr ext_hdr; | ^~~~~~~ Cc: [email protected] Buglink: https://bugs.launchpad.net/qemu/+bug/1879531 Reported-by: Alexander Bulekov <[email protected]> Reported-by: Miroslav Rezanina <[email protected]> Reviewed-by: Stefano Garzarella <[email protected]> Reviewed-by: Miroslav Rezanina <[email protected]> Fixes: eb70002 ("net_pkt: Extend packet abstraction as required by e1000e functionality") Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Signed-off-by: Jason Wang <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Apr 5, 2021
Incoming enabled bitmaps are busy, because we do bdrv_dirty_bitmap_create_successor() for them. But disabled bitmaps being migrated are not marked busy, and user can remove them during the incoming migration. Then we may crash in cancel_incoming_locked() when try to remove the bitmap that was already removed by user, like this: #0 qemu_mutex_lock_impl (mutex=0x5593d88c50d1, file=0x559680554b20 "../block/dirty-bitmap.c", line=64) at ../util/qemu-thread-posix.c:77 #1 bdrv_dirty_bitmaps_lock (bs=0x5593d88c0ee9) at ../block/dirty-bitmap.c:64 #2 bdrv_release_dirty_bitmap (bitmap=0x5596810e9570) at ../block/dirty-bitmap.c:362 #3 cancel_incoming_locked (s=0x559680be8208 <dbm_state+40>) at ../migration/block-dirty-bitmap.c:918 #4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0 <dbm_state>, version_id=1) at ../migration/block-dirty-bitmap.c:1194 #5 vmstate_load (f=0x559681d02b10, se=0x559680fb5810) at ../migration/savevm.c:908 #6 qemu_loadvm_section_part_end (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2473 #7 qemu_loadvm_state_main (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2626 #8 postcopy_ram_listen_thread (opaque=0x0) at ../migration/savevm.c:1871 #9 qemu_thread_start (args=0x5596817ccd10) at ../util/qemu-thread-posix.c:521 #10 start_thread () at /lib64/libpthread.so.0 #11 clone () at /lib64/libc.so.6 Note bs pointer taken from bitmap: it's definitely bad aligned. That's because we are in use after free, bitmap is already freed. So, let's make disabled bitmaps (being migrated) busy during incoming migration. Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> Signed-off-by: Stefan Hajnoczi <[email protected]> Message-Id: <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Apr 5, 2021
When building with --enable-sanitizers we get: Direct leak of 32 byte(s) in 2 object(s) allocated from: #0 0x5618479ec7cf in malloc (qemu-system-aarch64+0x233b7cf) #1 0x7f675745f958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958) #2 0x561847f02ca2 in usb_packet_init hw/usb/core.c:531:5 #3 0x561848df4df4 in usb_ehci_init hw/usb/hcd-ehci.c:2575:5 #4 0x561847c119ac in ehci_sysbus_init hw/usb/hcd-ehci-sysbus.c:73:5 #5 0x56184a5bdab8 in object_init_with_type qom/object.c:375:9 #6 0x56184a5bd955 in object_init_with_type qom/object.c:371:9 #7 0x56184a5a2bda in object_initialize_with_type qom/object.c:517:5 #8 0x56184a5a24d5 in object_initialize qom/object.c:536:5 #9 0x56184a5a2f6c in object_initialize_child_with_propsv qom/object.c:566:5 #10 0x56184a5a2e60 in object_initialize_child_with_props qom/object.c:549:10 #11 0x56184a5a3a1e in object_initialize_child_internal qom/object.c:603:5 #12 0x561849542d18 in npcm7xx_init hw/arm/npcm7xx.c:427:5 Similarly to commit d710e1e ("usb: ehci: fix memory leak in ehci"), fix by calling usb_ehci_finalize() to free the USBPacket. Fixes: 7341ea0 Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Thomas Huth <[email protected]> Message-Id: <[email protected]> Signed-off-by: Gerd Hoffmann <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Apr 5, 2021
When building with --enable-sanitizers we get: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x5618479ec7cf in malloc (qemu-system-aarch64+0x233b7cf) #1 0x7f675745f958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958) #2 0x561847c2dcc9 in xlnx_dp_init hw/display/xlnx_dp.c:1259:5 #3 0x56184a5bdab8 in object_init_with_type qom/object.c:375:9 #4 0x56184a5a2bda in object_initialize_with_type qom/object.c:517:5 #5 0x56184a5a24d5 in object_initialize qom/object.c:536:5 #6 0x56184a5a2f6c in object_initialize_child_with_propsv qom/object.c:566:5 #7 0x56184a5a2e60 in object_initialize_child_with_props qom/object.c:549:10 #8 0x56184a5a3a1e in object_initialize_child_internal qom/object.c:603:5 #9 0x5618495aa431 in xlnx_zynqmp_init hw/arm/xlnx-zynqmp.c:273:5 The RX/TX FIFOs are created in xlnx_dp_init(), add xlnx_dp_finalize() to destroy them. Fixes: 58ac482 ("introduce xlnx-dp") Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Alistair Francis <[email protected]> Message-id: [email protected] Signed-off-by: Peter Maydell <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Jul 21, 2021
Incoming enabled bitmaps are busy, because we do bdrv_dirty_bitmap_create_successor() for them. But disabled bitmaps being migrated are not marked busy, and user can remove them during the incoming migration. Then we may crash in cancel_incoming_locked() when try to remove the bitmap that was already removed by user, like this: #0 qemu_mutex_lock_impl (mutex=0x5593d88c50d1, file=0x559680554b20 "../block/dirty-bitmap.c", line=64) at ../util/qemu-thread-posix.c:77 #1 bdrv_dirty_bitmaps_lock (bs=0x5593d88c0ee9) at ../block/dirty-bitmap.c:64 #2 bdrv_release_dirty_bitmap (bitmap=0x5596810e9570) at ../block/dirty-bitmap.c:362 #3 cancel_incoming_locked (s=0x559680be8208 <dbm_state+40>) at ../migration/block-dirty-bitmap.c:918 #4 dirty_bitmap_load (f=0x559681d02b10, opaque=0x559680be81e0 <dbm_state>, version_id=1) at ../migration/block-dirty-bitmap.c:1194 #5 vmstate_load (f=0x559681d02b10, se=0x559680fb5810) at ../migration/savevm.c:908 #6 qemu_loadvm_section_part_end (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2473 #7 qemu_loadvm_state_main (f=0x559681d02b10, mis=0x559680fb4a30) at ../migration/savevm.c:2626 #8 postcopy_ram_listen_thread (opaque=0x0) at ../migration/savevm.c:1871 #9 qemu_thread_start (args=0x5596817ccd10) at ../util/qemu-thread-posix.c:521 #10 start_thread () at /lib64/libpthread.so.0 #11 clone () at /lib64/libc.so.6 Note bs pointer taken from bitmap: it's definitely bad aligned. That's because we are in use after free, bitmap is already freed. So, let's make disabled bitmaps (being migrated) busy during incoming migration. Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> Signed-off-by: Stefan Hajnoczi <[email protected]> Message-Id: <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Jul 21, 2021
When building with --enable-sanitizers we get: Direct leak of 32 byte(s) in 2 object(s) allocated from: #0 0x5618479ec7cf in malloc (qemu-system-aarch64+0x233b7cf) #1 0x7f675745f958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958) #2 0x561847f02ca2 in usb_packet_init hw/usb/core.c:531:5 #3 0x561848df4df4 in usb_ehci_init hw/usb/hcd-ehci.c:2575:5 #4 0x561847c119ac in ehci_sysbus_init hw/usb/hcd-ehci-sysbus.c:73:5 #5 0x56184a5bdab8 in object_init_with_type qom/object.c:375:9 #6 0x56184a5bd955 in object_init_with_type qom/object.c:371:9 #7 0x56184a5a2bda in object_initialize_with_type qom/object.c:517:5 #8 0x56184a5a24d5 in object_initialize qom/object.c:536:5 #9 0x56184a5a2f6c in object_initialize_child_with_propsv qom/object.c:566:5 #10 0x56184a5a2e60 in object_initialize_child_with_props qom/object.c:549:10 #11 0x56184a5a3a1e in object_initialize_child_internal qom/object.c:603:5 #12 0x561849542d18 in npcm7xx_init hw/arm/npcm7xx.c:427:5 Similarly to commit d710e1e ("usb: ehci: fix memory leak in ehci"), fix by calling usb_ehci_finalize() to free the USBPacket. Fixes: 7341ea0 Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Thomas Huth <[email protected]> Message-Id: <[email protected]> Signed-off-by: Gerd Hoffmann <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Jul 21, 2021
When building with --enable-sanitizers we get: Direct leak of 16 byte(s) in 1 object(s) allocated from: #0 0x5618479ec7cf in malloc (qemu-system-aarch64+0x233b7cf) #1 0x7f675745f958 in g_malloc (/lib64/libglib-2.0.so.0+0x58958) #2 0x561847c2dcc9 in xlnx_dp_init hw/display/xlnx_dp.c:1259:5 #3 0x56184a5bdab8 in object_init_with_type qom/object.c:375:9 #4 0x56184a5a2bda in object_initialize_with_type qom/object.c:517:5 #5 0x56184a5a24d5 in object_initialize qom/object.c:536:5 #6 0x56184a5a2f6c in object_initialize_child_with_propsv qom/object.c:566:5 #7 0x56184a5a2e60 in object_initialize_child_with_props qom/object.c:549:10 #8 0x56184a5a3a1e in object_initialize_child_internal qom/object.c:603:5 #9 0x5618495aa431 in xlnx_zynqmp_init hw/arm/xlnx-zynqmp.c:273:5 The RX/TX FIFOs are created in xlnx_dp_init(), add xlnx_dp_finalize() to destroy them. Fixes: 58ac482 ("introduce xlnx-dp") Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Alistair Francis <[email protected]> Message-id: [email protected] Signed-off-by: Peter Maydell <[email protected]>
cupertinomiranda
pushed a commit
that referenced
this issue
Jul 21, 2021
g_hash_table_add always retains ownership of the pointer passed in as the key. Its return status merely indicates whether the added entry was new, or replaced an existing entry. Thus key must never be freed after this method returns. Spotted by ASAN: ==2407186==ERROR: AddressSanitizer: heap-use-after-free on address 0x6020003ac4f0 at pc 0x7ffff766659c bp 0x7fffffffd1d0 sp 0x7fffffffc980 READ of size 1 at 0x6020003ac4f0 thread T0 #0 0x7ffff766659b (/lib64/libasan.so.6+0x8a59b) #1 0x7ffff6bfa843 in g_str_equal ../glib/ghash.c:2303 #2 0x7ffff6bf8167 in g_hash_table_lookup_node ../glib/ghash.c:493 #3 0x7ffff6bf9b78 in g_hash_table_insert_internal ../glib/ghash.c:1598 #4 0x7ffff6bf9c32 in g_hash_table_add ../glib/ghash.c:1689 #5 0x5555596caad4 in module_load_one ../util/module.c:233 #6 0x5555596ca949 in module_load_one ../util/module.c:225 #7 0x5555596ca949 in module_load_one ../util/module.c:225 #8 0x5555596cbdf4 in module_load_qom_all ../util/module.c:349 Typical C bug... Fixes: 9062912 ("module: use g_hash_table_add()") Cc: [email protected] Signed-off-by: Marc-André Lureau <[email protected]> Reviewed-by: Daniel P. Berrangé <[email protected]> Message-Id: <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
We can fail the blk_insert_bs() at init_blk_migration(), leaving the BlkMigDevState without a dirty_bitmap and BlockDriverState. Account for the possibly missing elements when doing cleanup. Fix the following crashes: Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 359 BlockDriverState *bs = bitmap->bs; #0 0x0000555555ec83ef in bdrv_release_dirty_bitmap (bitmap=0x0) at ../block/dirty-bitmap.c:359 #1 0x0000555555bba331 in unset_dirty_tracking () at ../migration/block.c:371 #2 0x0000555555bbad98 in block_migration_cleanup_bmds () at ../migration/block.c:681 Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 7073 QLIST_FOREACH_SAFE(blocker, &bs->op_blockers[op], list, next) { #0 0x0000555555e971ff in bdrv_op_unblock (bs=0x0, op=BLOCK_OP_TYPE_BACKUP_SOURCE, reason=0x0) at ../block.c:7073 #1 0x0000555555e9734a in bdrv_op_unblock_all (bs=0x0, reason=0x0) at ../block.c:7095 #2 0x0000555555bbae13 in block_migration_cleanup_bmds () at ../migration/block.c:690 Signed-off-by: Fabiano Rosas <[email protected]> Message-id: [email protected] Signed-off-by: Stefan Hajnoczi <[email protected]> (cherry picked from commit f187609) Signed-off-by: Michael Tokarev <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
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]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
Thread 1 "qemu-system-x86" received signal SIGSEGV, Segmentation fault. 0x0000555555888630 in dpy_ui_info_supported (con=0x0) at ../ui/console.c:812 812 return con->hw_ops->ui_info != NULL; (gdb) bt #0 0x0000555555888630 in dpy_ui_info_supported (con=0x0) at ../ui/console.c:812 #1 0x00005555558a44b1 in protocol_client_msg (vs=0x5555578c76c0, data=0x5555581e93f0 <incomplete sequence \373>, len=24) at ../ui/vnc.c:2585 #2 0x00005555558a19ac in vnc_client_read (vs=0x5555578c76c0) at ../ui/vnc.c:1607 #3 0x00005555558a1ac2 in vnc_client_io (ioc=0x5555581eb0e0, condition=G_IO_IN, opaque=0x5555578c76c0) at ../ui/vnc.c:1635 Fixes: https://issues.redhat.com/browse/RHEL-2600 Signed-off-by: Marc-André Lureau <[email protected]> Reviewed-by: Albert Esteve <[email protected]> (cherry picked from commit 48a35e1) Signed-off-by: Michael Tokarev <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
Replace the return path retry logic with finishing and restarting the thread. This fixes a race when resuming the migration that leads to a segfault. Currently when doing postcopy we consider that an IO error on the return path file could be due to a network intermittency. We then keep the thread alive but have it do cleanup of the 'from_dst_file' and wait on the 'postcopy_pause_rp' semaphore. When the user issues a migrate resume, a new return path is opened and the thread is allowed to continue. There's a race condition in the above mechanism. It is possible for the new return path file to be setup *before* the cleanup code in the return path thread has had a chance to run, leading to the *new* file being closed and the pointer set to NULL. When the thread is released after the resume, it tries to dereference 'from_dst_file' and crashes: Thread 7 "return path" received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffd1dbf700 (LWP 9611)] 0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154 154 return f->last_error; (gdb) bt #0 0x00005555560e4893 in qemu_file_get_error_obj (f=0x0, errp=0x0) at ../migration/qemu-file.c:154 #1 0x00005555560e4983 in qemu_file_get_error (f=0x0) at ../migration/qemu-file.c:206 #2 0x0000555555b9a1df in source_return_path_thread (opaque=0x555556e06000) at ../migration/migration.c:1876 #3 0x000055555602e14f in qemu_thread_start (args=0x55555782e780) at ../util/qemu-thread-posix.c:541 #4 0x00007ffff38d76ea in start_thread (arg=0x7fffd1dbf700) at pthread_create.c:477 #5 0x00007ffff35efa6f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 Here's the race (important bit is open_return_path happening before migration_release_dst_files): migration | qmp | return path --------------------------+-----------------------------+--------------------------------- qmp_migrate_pause() shutdown(ms->to_dst_file) f->last_error = -EIO migrate_detect_error() postcopy_pause() set_state(PAUSED) wait(postcopy_pause_sem) qmp_migrate(resume) migrate_fd_connect() resume = state == PAUSED open_return_path <-- TOO SOON! set_state(RECOVER) post(postcopy_pause_sem) (incoming closes to_src_file) res = qemu_file_get_error(rp) migration_release_dst_files() ms->rp_state.from_dst_file = NULL post(postcopy_pause_rp_sem) postcopy_pause_return_path_thread() wait(postcopy_pause_rp_sem) rp = ms->rp_state.from_dst_file goto retry qemu_file_get_error(rp) SIGSEGV ------------------------------------------------------------------------------------------- We can keep the retry logic without having the thread alive and waiting. The only piece of data used by it is the 'from_dst_file' and it is only allowed to proceed after a migrate resume is issued and the semaphore released at migrate_fd_connect(). Move the retry logic to outside the thread by waiting for the thread to finish before pausing the migration. Reviewed-by: Peter Xu <[email protected]> Signed-off-by: Fabiano Rosas <[email protected]> Signed-off-by: Stefan Hajnoczi <[email protected]> Message-ID: <[email protected]> (cherry picked from commit ef796ee) Signed-off-by: Michael Tokarev <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
If there is a pending DMA operation during ide_bus_reset(), the fact that the IDEState is already reset before the operation is canceled can be problematic. In particular, ide_dma_cb() might be called and then use the reset IDEState which contains the signature after the reset. When used to construct the IO operation this leads to ide_get_sector() returning 0 and nsector being 1. This is particularly bad, because a write command will thus destroy the first sector which often contains a partition table or similar. Traces showing the unsolicited write happening with IDEState 0x5595af6949d0 being used after reset: > ahci_port_write ahci(0x5595af6923f0)[0]: port write [reg:PxSCTL] @ 0x2c: 0x00000300 > ahci_reset_port ahci(0x5595af6923f0)[0]: reset port > ide_reset IDEstate 0x5595af6949d0 > ide_reset IDEstate 0x5595af694da8 > ide_bus_reset_aio aio_cancel > dma_aio_cancel dbs=0x7f64600089a0 > dma_blk_cb dbs=0x7f64600089a0 ret=0 > dma_complete dbs=0x7f64600089a0 ret=0 cb=0x5595acd40b30 > ahci_populate_sglist ahci(0x5595af6923f0)[0] > ahci_dma_prepare_buf ahci(0x5595af6923f0)[0]: prepare buf limit=512 prepared=512 > ide_dma_cb IDEState 0x5595af6949d0; sector_num=0 n=1 cmd=DMA WRITE > dma_blk_io dbs=0x7f6420802010 bs=0x5595ae2c6c30 offset=0 to_dev=1 > dma_blk_cb dbs=0x7f6420802010 ret=0 > (gdb) p *qiov > $11 = {iov = 0x7f647c76d840, niov = 1, {{nalloc = 1, local_iov = {iov_base = 0x0, > iov_len = 512}}, {__pad = "\001\000\000\000\000\000\000\000\000\000\000", > size = 512}}} > (gdb) bt > #0 blk_aio_pwritev (blk=0x5595ae2c6c30, offset=0, qiov=0x7f6420802070, flags=0, > cb=0x5595ace6f0b0 <dma_blk_cb>, opaque=0x7f6420802010) > at ../block/block-backend.c:1682 > #1 0x00005595ace6f185 in dma_blk_cb (opaque=0x7f6420802010, ret=<optimized out>) > at ../softmmu/dma-helpers.c:179 > #2 0x00005595ace6f778 in dma_blk_io (ctx=0x5595ae0609f0, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > io_func=io_func@entry=0x5595ace6ee30 <dma_blk_write_io_func>, > io_func_opaque=io_func_opaque@entry=0x5595ae2c6c30, > cb=0x5595acd40b30 <ide_dma_cb>, opaque=0x5595af6949d0, > dir=DMA_DIRECTION_TO_DEVICE) at ../softmmu/dma-helpers.c:244 > #3 0x00005595ace6f90a in dma_blk_write (blk=0x5595ae2c6c30, > sg=sg@entry=0x5595af694d00, offset=offset@entry=0, align=align@entry=512, > cb=cb@entry=0x5595acd40b30 <ide_dma_cb>, opaque=opaque@entry=0x5595af6949d0) > at ../softmmu/dma-helpers.c:280 > #4 0x00005595acd40e18 in ide_dma_cb (opaque=0x5595af6949d0, ret=<optimized out>) > at ../hw/ide/core.c:953 > #5 0x00005595ace6f319 in dma_complete (ret=0, dbs=0x7f64600089a0) > at ../softmmu/dma-helpers.c:107 > #6 dma_blk_cb (opaque=0x7f64600089a0, ret=0) at ../softmmu/dma-helpers.c:127 > #7 0x00005595ad12227d in blk_aio_complete (acb=0x7f6460005b10) > at ../block/block-backend.c:1527 > #8 blk_aio_complete (acb=0x7f6460005b10) at ../block/block-backend.c:1524 > #9 blk_aio_write_entry (opaque=0x7f6460005b10) at ../block/block-backend.c:1594 > #10 0x00005595ad258cfb in coroutine_trampoline (i0=<optimized out>, > i1=<optimized out>) at ../util/coroutine-ucontext.c:177 Signed-off-by: Fiona Ebner <[email protected]> Reviewed-by: Philippe Mathieu-Daudé <[email protected]> Tested-by: [email protected] Message-ID: <[email protected]> Signed-off-by: Philippe Mathieu-Daudé <[email protected]> (cherry picked from commit 7d75120) Signed-off-by: Michael Tokarev <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
…ock_status Using fleecing backup like in [0] on a qcow2 image (with metadata preallocation) can lead to the following assertion failure: > bdrv_co_do_block_status: Assertion `!(ret & BDRV_BLOCK_ZERO)' failed. In the reproducer [0], it happens because the BDRV_BLOCK_RECURSE flag will be set by the qcow2 driver, so the caller will recursively check the file child. Then the BDRV_BLOCK_ZERO set too. Later up the call chain, in bdrv_co_do_block_status() for the snapshot-access driver, the assertion failure will happen, because both flags are set. To fix it, clear the recurse flag after the recursive check was done. In detail: > #0 qcow2_co_block_status Returns 0x45 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID. > #1 bdrv_co_do_block_status Because of the data flag, bdrv_co_do_block_status() will now also set BDRV_BLOCK_ALLOCATED. Because of the recurse flag, bdrv_co_do_block_status() for the bdrv_file child will be called, which returns 0x16 = BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ZERO. Now the return value inherits the zero flag. Returns 0x57 = BDRV_BLOCK_RECURSE | BDRV_BLOCK_DATA | BDRV_BLOCK_OFFSET_VALID | BDRV_BLOCK_ALLOCATED | BDRV_BLOCK_ZERO. > #2 bdrv_co_common_block_status_above > #3 bdrv_co_block_status_above > #4 bdrv_co_block_status > #5 cbw_co_snapshot_block_status > #6 bdrv_co_snapshot_block_status > #7 snapshot_access_co_block_status > #8 bdrv_co_do_block_status Return value is propagated all the way up to here, where the assertion failure happens, because BDRV_BLOCK_RECURSE and BDRV_BLOCK_ZERO are both set. > #9 bdrv_co_common_block_status_above > #10 bdrv_co_block_status_above > #11 block_copy_block_status > #12 block_copy_dirty_clusters > #13 block_copy_common > #14 block_copy_async_co_entry > #15 coroutine_trampoline [0]: > #!/bin/bash > rm /tmp/disk.qcow2 > ./qemu-img create /tmp/disk.qcow2 -o preallocation=metadata -f qcow2 1G > ./qemu-img create /tmp/fleecing.qcow2 -f qcow2 1G > ./qemu-img create /tmp/backup.qcow2 -f qcow2 1G > ./qemu-system-x86_64 --qmp stdio \ > --blockdev qcow2,node-name=node0,file.driver=file,file.filename=/tmp/disk.qcow2 \ > --blockdev qcow2,node-name=node1,file.driver=file,file.filename=/tmp/fleecing.qcow2 \ > --blockdev qcow2,node-name=node2,file.driver=file,file.filename=/tmp/backup.qcow2 \ > <<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": "blockdev-backup", "arguments": { "device": "snap0", "target": "node1", "sync": "full", "job-id": "backup0" } } > EOF Signed-off-by: Fiona Ebner <[email protected]> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]> Message-id: [email protected] Signed-off-by: Stefan Hajnoczi <[email protected]> (cherry picked from commit 8a9be79) Signed-off-by: Michael Tokarev <[email protected]>
kolerov
pushed a commit
that referenced
this issue
Jul 13, 2024
There is a bug in the blklogwrites driver pertaining to logging "write zeroes" operations, causing log corruption. This can be easily observed by setting detect-zeroes to something other than "off" for the driver. The issue is caused by a concurrency bug pertaining to the fact that "write zeroes" operations have to be logged in two parts: first the log entry metadata, then the zeroed-out region. While the log entry metadata is being written by bdrv_co_pwritev(), another operation may begin in the meanwhile and modify the state of the blklogwrites driver. This is as intended by the coroutine-driven I/O model in QEMU, of course. Unfortunately, this specific scenario is mishandled. A short example: 1. Initially, in the current operation (#1), the current log sector number in the driver state is only incremented by the number of sectors taken by the log entry metadata, after which the log entry metadata is written. The current operation yields. 2. Another operation (#2) may start while the log entry metadata is being written. It uses the current log position as the start offset for its log entry. This is in the sector right after the operation #1 log entry metadata, which is bad! 3. After bdrv_co_pwritev() returns (#1), the current log sector number is reread from the driver state in order to find out the start offset for bdrv_co_pwrite_zeroes(). This is an obvious blunder, as the offset will be the sector right after the (misplaced) operation #2 log entry, which means that the zeroed-out region begins at the wrong offset. 4. As a result of the above, the log is corrupt. Fix this by only reading the driver metadata once, computing the offsets and sizes in one go (including the optional zeroed-out region) and setting the log sector number to the appropriate value for the next operation in line. Signed-off-by: Ari Sundholm <[email protected]> Cc: [email protected] Message-ID: <[email protected]> Reviewed-by: Kevin Wolf <[email protected]> Signed-off-by: Kevin Wolf <[email protected]> (cherry picked from commit a9c8ea9) Signed-off-by: Michael Tokarev <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
ARConnect is a collection of system components that provides efficient inter-core communications,
interrupt distribution, debug assistance, and other inter-core functionality in a multi-core system.
See chapter "26 ARConnect Support" of ARC HS databook & chapter "36 ARConnect" of ARC HS PRM.
This issue is dedicated for implementation of a common command interface used by ARConnect.
The text was updated successfully, but these errors were encountered: