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.

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's Global Free-Running Counter (GFRC) #6

Open
abrodkin opened this issue Jun 3, 2019 · 0 comments
Open

Support ARConnect's Global Free-Running Counter (GFRC) #6

abrodkin opened this issue Jun 3, 2019 · 0 comments
Assignees
Labels
enhancement New feature or request SMP

Comments

@abrodkin
Copy link
Member

abrodkin commented Jun 3, 2019

The Global Free-Running Counter Unit (GFRC) implements a 64-bit free-running global real-time counter that can be used as a central timing reference when multiple cores are under different clock domains.

See chapter "26.7 Global Free-Running Counter" of ARC HS databook.

Similarly to IDU this block might be used in UP (Uni-Processor) system so might be implemented and tested in existing setups.

@abrodkin abrodkin changed the title Support GFRC (Global Free-Running Counter) Support ARConnect's Global Free-Running Counter (GFRC) Jun 3, 2019
@vineetgarc vineetgarc transferred this issue from another repository Nov 19, 2020
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]>
@claziss claziss added the enhancement New feature or request label Jul 2, 2021
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 kolerov assigned kolerov and unassigned BrunoASMauricio Jul 21, 2023
@kolerov kolerov added the SMP label Jul 2, 2024
kolerov pushed a commit that referenced this issue Jul 13, 2024
Otherwise tcg_handle_interrupt() triggers an assertion failure:

  #5  0x0000555555c97369 in tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:83
  #6  tcg_handle_interrupt (cpu=0x555557434cb0, mask=2) at ../accel/tcg/tcg-accel-ops.c:81
  #7  0x0000555555b4d58b in pic_irq_request (opaque=<optimized out>, irq=<optimized out>, level=1) at ../hw/i386/x86.c:555
  #8  0x0000555555b4f218 in gsi_handler (opaque=0x5555579423d0, n=13, level=1) at ../hw/i386/x86.c:611
  #9  0x00007fffa42bde14 in code_gen_buffer ()
  #10 0x0000555555c724bb in cpu_tb_exec (cpu=cpu@entry=0x555557434cb0, itb=<optimized out>, tb_exit=tb_exit@entry=0x7fffe9bfd658) at ../accel/tcg/cpu-exec.c:457

Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1808
Reported-by: NyanCatTW1 <https://gitlab.com/a0939712328>
Co-developed-by: Richard Henderson <[email protected]>'
Reviewed-by: Philippe Mathieu-Daudé <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
(cherry picked from commit c1f27a0)
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
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
There is no architectural requirement that SME implies SVE, but
our implementation currently assumes it. (FEAT_SME_FA64 does
imply SVE.) So if you try to run a CPU with eg "-cpu max,sve=off"
you quickly run into an assert when the guest tries to write to
SMCR_EL1:

#6  0x00007ffff4b38e96 in __GI___assert_fail
    (assertion=0x5555566e69cb "sm", file=0x5555566e5b24 "../../target/arm/helper.c", line=6865, function=0x5555566e82f0 <__PRETTY_FUNCTION__.31> "sve_vqm1_for_el_sm") at ./assert/assert.c:101
#7  0x0000555555ee33aa in sve_vqm1_for_el_sm (env=0x555557d291f0, el=2, sm=false) at ../../target/arm/helper.c:6865
#8  0x0000555555ee3407 in sve_vqm1_for_el (env=0x555557d291f0, el=2) at ../../target/arm/helper.c:6871
#9  0x0000555555ee3724 in smcr_write (env=0x555557d291f0, ri=0x555557da23b0, value=2147483663) at ../../target/arm/helper.c:6995
#10 0x0000555555fd1dba in helper_set_cp_reg64 (env=0x555557d291f0, rip=0x555557da23b0, value=2147483663) at ../../target/arm/tcg/op_helper.c:839
#11 0x00007fff60056781 in code_gen_buffer ()

Avoid this unsupported and slightly odd combination by
disabling SME when SVE is not present.

Cc: [email protected]
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2005
Signed-off-by: Peter Maydell <[email protected]>
Reviewed-by: Richard Henderson <[email protected]>
Message-id: [email protected]
(cherry picked from commit f7767ca)
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
Labels
enhancement New feature or request SMP
Projects
None yet
Development

No branches or pull requests

6 participants