1. 15 5月, 2019 2 次提交
    • Y
      migration: Fix use-after-free during process exit · fd392cfa
      Yury Kotov 提交于
      It fixes heap-use-after-free which was found by clang's ASAN.
      
      Control flow of this use-after-free:
      main_thread:
          * Got SIGTERM and completes main loop
          * Calls migration_shutdown
            - migrate_fd_cancel (so, migration_thread begins to complete)
            - object_unref(OBJECT(current_migration));
      
      migration_thread:
          * migration_iteration_finish -> schedule cleanup bh
          * object_unref(OBJECT(s)); (Now, current_migration is freed)
          * exits
      
      main_thread:
          * Calls vm_shutdown -> drain bdrvs -> main loop
            -> cleanup_bh -> use after free
      
      If you want to reproduce, these couple of sleeps will help:
      vl.c:4613:
           migration_shutdown();
      +    sleep(2);
      migration.c:3269:
      +    sleep(1);
           trace_migration_thread_after_loop();
           migration_iteration_finish(s);
      
      Original output:
      qemu-system-x86_64: terminating on signal 15 from pid 31980 (<unknown process>)
      =================================================================
      ==31958==ERROR: AddressSanitizer: heap-use-after-free on address 0x61900001d210
        at pc 0x555558a535ca bp 0x7fffffffb190 sp 0x7fffffffb188
      READ of size 8 at 0x61900001d210 thread T0 (qemu-vm-0)
          #0 0x555558a535c9 in migrate_fd_cleanup migration/migration.c:1502:23
          #1 0x5555594fde0a in aio_bh_call util/async.c:90:5
          #2 0x5555594fe522 in aio_bh_poll util/async.c:118:13
          #3 0x555559524783 in aio_poll util/aio-posix.c:725:17
          #4 0x555559504fb3 in aio_wait_bh_oneshot util/aio-wait.c:71:5
          #5 0x5555573bddf6 in virtio_blk_data_plane_stop
            hw/block/dataplane/virtio-blk.c:282:5
          #6 0x5555589d5c09 in virtio_bus_stop_ioeventfd hw/virtio/virtio-bus.c:246:9
          #7 0x5555589e9917 in virtio_pci_stop_ioeventfd hw/virtio/virtio-pci.c:287:5
          #8 0x5555589e22bf in virtio_pci_vmstate_change hw/virtio/virtio-pci.c:1072:9
          #9 0x555557628931 in virtio_vmstate_change hw/virtio/virtio.c:2257:9
          #10 0x555557c36713 in vm_state_notify vl.c:1605:9
          #11 0x55555716ef53 in do_vm_stop cpus.c:1074:9
          #12 0x55555716eeff in vm_shutdown cpus.c:1092:12
          #13 0x555557c4283e in main vl.c:4617:5
          #14 0x7fffdfdb482f in __libc_start_main
            (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
          #15 0x555556ecb118 in _start (x86_64-softmmu/qemu-system-x86_64+0x1977118)
      
      0x61900001d210 is located 144 bytes inside of 952-byte region
        [0x61900001d180,0x61900001d538)
      freed by thread T6 (live_migration) here:
          #0 0x555556f76782 in __interceptor_free
            /tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:124:3
          #1 0x555558d5fa94 in object_finalize qom/object.c:618:9
          #2 0x555558d57651 in object_unref qom/object.c:1068:9
          #3 0x555558a55588 in migration_thread migration/migration.c:3272:5
          #4 0x5555595393f2 in qemu_thread_start util/qemu-thread-posix.c:502:9
          #5 0x7fffe057f6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
      
      previously allocated by thread T0 (qemu-vm-0) here:
          #0 0x555556f76b03 in __interceptor_malloc
            /tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:146:3
          #1 0x7ffff6ee37b8 in g_malloc (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4f7b8)
          #2 0x555558d58031 in object_new qom/object.c:640:12
          #3 0x555558a31f21 in migration_object_init migration/migration.c:139:25
          #4 0x555557c41398 in main vl.c:4320:5
          #5 0x7fffdfdb482f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
      
      Thread T6 (live_migration) created by T0 (qemu-vm-0) here:
          #0 0x555556f5f0dd in pthread_create
            /tmp/final/llvm.src/projects/compiler-rt/lib/asan/asan_interceptors.cc:210:3
          #1 0x555559538cf9 in qemu_thread_create util/qemu-thread-posix.c:539:11
          #2 0x555558a53304 in migrate_fd_connect migration/migration.c:3332:5
          #3 0x555558a72bd8 in migration_channel_connect migration/channel.c:92:5
          #4 0x555558a6ef87 in exec_start_outgoing_migration migration/exec.c:42:5
          #5 0x555558a4f3c2 in qmp_migrate migration/migration.c:1922:9
          #6 0x555558bb4f6a in qmp_marshal_migrate qapi/qapi-commands-migration.c:607:5
          #7 0x555559363738 in do_qmp_dispatch qapi/qmp-dispatch.c:131:5
          #8 0x555559362a15 in qmp_dispatch qapi/qmp-dispatch.c:174:11
          #9 0x5555571bac15 in monitor_qmp_dispatch monitor.c:4124:11
          #10 0x55555719a22d in monitor_qmp_bh_dispatcher monitor.c:4207:9
          #11 0x5555594fde0a in aio_bh_call util/async.c:90:5
          #12 0x5555594fe522 in aio_bh_poll util/async.c:118:13
          #13 0x5555595201e0 in aio_dispatch util/aio-posix.c:460:5
          #14 0x555559503553 in aio_ctx_dispatch util/async.c:261:5
          #15 0x7ffff6ede196 in g_main_context_dispatch
            (/lib/x86_64-linux-gnu/libglib-2.0.so.0+0x4a196)
      
      SUMMARY: AddressSanitizer: heap-use-after-free migration/migration.c:1502:23
        in migrate_fd_cleanup
      Shadow bytes around the buggy address:
        0x0c327fffb9f0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
        0x0c327fffba00: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
        0x0c327fffba10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
        0x0c327fffba20: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
        0x0c327fffba30: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      =>0x0c327fffba40: fd fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c327fffba50: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c327fffba60: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c327fffba70: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c327fffba80: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
        0x0c327fffba90: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
      Shadow byte legend (one shadow byte represents 8 application bytes):
        Addressable: 00
        Partially addressable: 01 02 03 04 05 06 07
        Heap left redzone: fa
        Freed heap region: fd
        Stack left redzone: f1
        Stack mid redzone: f2
        Stack right redzone: f3
        Stack after return: f5
        Stack use after scope: f8
        Global redzone: f9
        Global init order: f6
        Poisoned by user: f7
        Container overflow: fc
        Array cookie: ac
        Intra object redzone: bb
        ASan internal: fe
        Left alloca redzone: ca
        Right alloca redzone: cb
        Shadow gap: cc
      ==31958==ABORTING
      Signed-off-by: NYury Kotov <yury-kotov@yandex-team.ru>
      Message-Id: <20190408113343.2370-1-yury-kotov@yandex-team.ru>
      Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      Signed-off-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
        Fixed up comment formatting
      fd392cfa
    • W
      migration: remove not used field xfer_limit · 15d2d64c
      Wei Yang 提交于
      MigrationState->xfer_limit is only set to 0 in migrate_init().
      
      Remove this unnecessary field.
      Signed-off-by: NWei Yang <richardw.yang@linux.intel.com>
      Message-Id: <20190326055726.10539-1-richardw.yang@linux.intel.com>
      Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      Signed-off-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      15d2d64c
  2. 02 4月, 2019 2 次提交
    • M
      migration: Support adding migration blockers earlier · daff7f0b
      Markus Armbruster 提交于
      migrate_add_blocker() asserts we have a current_migration object, in
      migrate_get_current().  We do only after migration_object_init().
      
      This contributes to the following dependency cycle:
      
      * configure_blockdev() must run before machine_set_property()
        so machine properties can refer to block backends
      
      * machine_set_property() before configure_accelerator()
        so machine properties like kvm-irqchip get applied
      
      * configure_accelerator() before migration_object_init()
        so that Xen's accelerator compat properties get applied.
      
      * migration_object_init() before configure_blockdev()
        so configure_blockdev() can add migration blockers
      
      The cycle was closed when recent commit cda4aa9a "Create block
      backends before setting machine properties" added the first
      dependency, and satisfied it by violating the last one.  Broke block
      backends that add migration blockers, as demonstrated by qemu-iotests
      055.
      
      To fix it, break the last dependency: make migrate_add_blocker()
      usable before migration_object_init().
      
      The previous commit already removed the use of migrate_get_current()
      from migrate_add_blocker() itself.  Didn't quite do the trick, as
      there's another one hiding in migration_is_idle().
      
      The use there isn't actually necessary: when no migration object has
      been created yet, migration is surely idle.  Make migration_is_idle()
      return true then.
      
      Fixes: cda4aa9aSigned-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20190401090827.20793-4-armbru@redhat.com>
      Reviewed-by: NIgor Mammedov <imammedo@redhat.com>
      daff7f0b
    • M
      Revert "migration: move only_migratable to MigrationState" · 811f8652
      Markus Armbruster 提交于
      This reverts commit 3df663e5.
      This reverts commit b605c47b.
      
      Command line option --only-migratable is for disallowing any
      configuration that can block migration.
      
      Initially, --only-migratable set global variable @only_migratable.
      
      Commit 3df663e5 "migration: move only_migratable to MigrationState"
      replaced it by MigrationState member @only_migratable.  That was a
      mistake.
      
      First, it doesn't make sense on the design level.  MigrationState
      captures the state of an individual migration, but --only-migratable
      isn't a property of an individual migration, it's a restriction on
      QEMU configuration.  With fault tolerance, we could have several
      migrations at once.  --only-migratable would certainly protect all of
      them.  Storing it in MigrationState feels inappropriate.
      
      Second, it contributes to a dependency cycle that manifests itself as
      a bug now.
      
      Putting @only_migratable into MigrationState means its available only
      after migration_object_init().
      
      We can't set it before migration_object_init(), so we delay setting it
      with a global property (this is fixup commit b605c47b "migration:
      fix handling for --only-migratable").
      
      We can't get it before migration_object_init(), so anything that uses
      it can only run afterwards.
      
      Since migrate_add_blocker() needs to obey --only-migratable, any code
      adding migration blockers can run only afterwards.  This contributes
      to the following dependency cycle:
      
      * configure_blockdev() must run before machine_set_property()
        so machine properties can refer to block backends
      
      * machine_set_property() before configure_accelerator()
        so machine properties like kvm-irqchip get applied
      
      * configure_accelerator() before migration_object_init()
        so that Xen's accelerator compat properties get applied.
      
      * migration_object_init() before configure_blockdev()
        so configure_blockdev() can add migration blockers
      
      The cycle was closed when recent commit cda4aa9a "Create block
      backends before setting machine properties" added the first
      dependency, and satisfied it by violating the last one.  Broke block
      backends that add migration blockers.
      
      Moving @only_migratable into MigrationState was a mistake.  Revert it.
      
      This doesn't quite break the "migration_object_init() before
      configure_blockdev() dependency, since migrate_add_blocker() still has
      another dependency on migration_object_init().  To be addressed the
      next commit.
      
      Note that the reverted commit made -only-migratable sugar for -global
      migration.only-migratable=on below the hood.  Documentation has only
      ever mentioned -only-migratable.  This commit removes the arcane &
      undocumented alternative to -only-migratable again.  Nobody should be
      using it.
      
      Conflicts:
      	include/migration/misc.h
      	migration/migration.c
      	migration/migration.h
      	vl.c
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      Message-Id: <20190401090827.20793-3-armbru@redhat.com>
      Reviewed-by: NIgor Mammedov <imammedo@redhat.com>
      811f8652
  3. 26 3月, 2019 4 次提交
  4. 06 3月, 2019 5 次提交
  5. 05 3月, 2019 3 次提交
  6. 23 1月, 2019 4 次提交
  7. 18 12月, 2018 1 次提交
    • D
      qmp hmp: Make system_wakeup check wake-up support and run state · fb064112
      Daniel Henrique Barboza 提交于
      The qmp/hmp command 'system_wakeup' is simply a direct call to
      'qemu_system_wakeup_request' from vl.c. This function verifies if
      runstate is SUSPENDED and if the wake up reason is valid before
      proceeding. However, no error or warning is thrown if any of those
      pre-requirements isn't met. There is no way for the caller to
      differentiate between a successful wakeup or an error state caused
      when trying to wake up a guest that wasn't suspended.
      
      This means that system_wakeup is silently failing, which can be
      considered a bug. Adding error handling isn't an API break in this
      case - applications that didn't check the result will remain broken,
      the ones that check it will have a chance to deal with it.
      
      Adding to that, the commit before previous created a new QMP API called
      query-current-machine, with a new flag called wakeup-suspend-support,
      that indicates if the guest has the capability of waking up from suspended
      state. Although such guest will never reach SUSPENDED state and erroring
      it out in this scenario would suffice, it is more informative for the user
      to differentiate between a failure because the guest isn't suspended versus
      a failure because the guest does not have support for wake up at all.
      
      All this considered, this patch changes qmp_system_wakeup to check if
      the guest is capable of waking up from suspend, and if it is suspended.
      After this patch, this is the output of system_wakeup in a guest that
      does not have wake-up from suspend support (ppc64):
      
      (qemu) system_wakeup
      wake-up from suspend is not supported by this guest
      (qemu)
      
      And this is the output of system_wakeup in a x86 guest that has the
      support but isn't suspended:
      
      (qemu) system_wakeup
      Unable to wake up: guest is not in suspended state
      (qemu)
      Reported-by: NBalamuruhan S <bala24@linux.vnet.ibm.com>
      Signed-off-by: NDaniel Henrique Barboza <danielhb413@gmail.com>
      Message-Id: <20181205194701.17836-4-danielhb413@gmail.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Acked-by: NEduardo Habkost <ehabkost@redhat.com>
      Reviewed-by: NMichael S. Tsirkin <mst@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      fb064112
  8. 21 11月, 2018 1 次提交
  9. 31 10月, 2018 1 次提交
  10. 19 10月, 2018 5 次提交
  11. 27 9月, 2018 1 次提交
    • M
      migration: fix QEMUFile leak · 0284a2a8
      Marc-André Lureau 提交于
      Spotted by ASAN while running:
      
      $ tests/migration-test -p /x86_64/migration/postcopy/recovery
      
      =================================================================
      ==18034==ERROR: LeakSanitizer: detected memory leaks
      
      Direct leak of 33864 byte(s) in 1 object(s) allocated from:
          #0 0x7f3da7f31e50 in calloc (/lib64/libasan.so.5+0xeee50)
          #1 0x7f3da644441d in g_malloc0 (/lib64/libglib-2.0.so.0+0x5241d)
          #2 0x55af9db15440 in qemu_fopen_channel_input /home/elmarco/src/qemu/migration/qemu-file-channel.c:183
          #3 0x55af9db15413 in channel_get_output_return_path /home/elmarco/src/qemu/migration/qemu-file-channel.c:159
          #4 0x55af9db0d4ac in qemu_file_get_return_path /home/elmarco/src/qemu/migration/qemu-file.c:78
          #5 0x55af9dad5e4f in open_return_path_on_source /home/elmarco/src/qemu/migration/migration.c:2295
          #6 0x55af9dadb3bf in migrate_fd_connect /home/elmarco/src/qemu/migration/migration.c:3111
          #7 0x55af9dae1bf3 in migration_channel_connect /home/elmarco/src/qemu/migration/channel.c:91
          #8 0x55af9daddeca in socket_outgoing_migration /home/elmarco/src/qemu/migration/socket.c:108
          #9 0x55af9e13d3db in qio_task_complete /home/elmarco/src/qemu/io/task.c:158
          #10 0x55af9e13ca03 in qio_task_thread_result /home/elmarco/src/qemu/io/task.c:89
          #11 0x7f3da643b1ca in g_idle_dispatch gmain.c:5535
      Signed-off-by: NMarc-André Lureau <marcandre.lureau@redhat.com>
      Message-Id: <20180925092245.29565-1-marcandre.lureau@redhat.com>
      Reviewed-by: NPeter Xu <peterx@redhat.com>
      Reviewed-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      Signed-off-by: NDr. David Alan Gilbert <dgilbert@redhat.com>
      0284a2a8
  12. 26 9月, 2018 1 次提交
  13. 29 8月, 2018 1 次提交
  14. 22 8月, 2018 4 次提交
  15. 25 7月, 2018 3 次提交
  16. 10 7月, 2018 2 次提交