qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PULL 2/5] migration: move bdrv_invalidate_cache_all of


From: Denis V. Lunev
Subject: Re: [Qemu-devel] [PULL 2/5] migration: move bdrv_invalidate_cache_all of of coroutine context
Date: Mon, 7 Mar 2016 21:58:29 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1

On 03/07/2016 03:49 PM, Dr. David Alan Gilbert wrote:
* Amit Shah (address@hidden) wrote:
From: "Denis V. Lunev" <address@hidden>

There is a possibility to hit an assert in qcow2_get_specific_info that
s->qcow_version is undefined. This happens when VM in starting from
suspended state, i.e. it processes incoming migration, and in the same
time 'info block' is called.

The problem is that qcow2_invalidate_cache() closes the image and
memset()s BDRVQcowState in the middle.

The patch moves processing of bdrv_invalidate_cache_all out of
coroutine context for postcopy migration to avoid that. This function
is called with the following stack:
   process_incoming_migration_co
   qemu_loadvm_state
   qemu_loadvm_state_main
   loadvm_process_command
   loadvm_postcopy_handle_run

Signed-off-by: Denis V. Lunev <address@hidden>
Tested-by: Dr. David Alan Gilbert <address@hidden>
hmm; actually - this segs in a variety of different ways;
there are two problems:

    a) +    bh = qemu_bh_new(loadvm_postcopy_handle_run_bh, NULL);
      That's the easy one; that NULL should be 'mis', because
      the bh is expecting to use it as a MigrationIncomingState
      so it segs fairly reliably in the qemu_bh_delete(mis->bh)

    b) The harder problem is that there's a race where qemu_bh_delete
       segs, and I'm not 100% sure why yet - it only does it sometime
       (i.e. run virt-test and leave it and it occasionally does it).
       From the core it looks like qemu->bh is corrupt (0x10101010...)
       so maybe mis has been freed at that point?
       I'm suspecting this is the postcopy_ram_listen_thread freeing
       mis at the end of it, but I don't know yet.

Dave

Yes. this is exactly use-after-free. I have looked into the code
and this seems correct.

Could you try this simple patch?

Den


Attachment: 1.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]