Support Inter-Core Debug Unit (ICD) #4
Assignees
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]>
cupertinomiranda
pushed a commit
that referenced
this issue
Jul 21, 2021
When adding the Reset register in commit 5790b75 we forgot to migrate it. While it is possible a VM using the PIIX4 is migrated just after requesting a system shutdown, it is very unlikely. However when restoring a migrated VM, we might have the RCR bit #4 set on the stack and when the VM resume it directly shutdowns. Add a post_load() migration handler and set the default RCR value to 0 for earlier versions, assuming the VM was not going to shutdown before migration. Fixes: 5790b75 ("piix4: Add the Reset Control Register") Signed-off-by: Philippe Mathieu-Daudé <[email protected]> Reviewed-by: Dr. David Alan Gilbert <[email protected]> Message-Id: <[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
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]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
The Inter-core Debug Unit (ICD) provides additional debug assist features in multi-core scenarios.
For example hall all cores if either of them got halted etc.
See chapter "26.6 Inter-Core Debug Unit" in ARC HS databook.
This feature might be to some extent implemented and tested on UP (Uni-Processor) setup.
The text was updated successfully, but these errors were encountered: