Re: [Qemu-devel] [PATCH v2 1/2] migration: don't close a file descriptor while it can be in use

[Dr. David Alan Gilbert]

* Laurent Vivier (address@hidden) wrote:
> If we close the QEMUFile descriptor in process_incoming_migration_co()
> while it has been stopped by an error, the postcopy_ram_listen_thread()
> can try to continue to use it. And as the memory has been freed
> it is working with an invalid pointer and crashes.
> 
> Fix this by releasing the memory after having managed the error
> case (which, in fact, calls exit())
> 
> Signed-off-by: Laurent Vivier <address@hidden>

Reviewed-by: Dr. David Alan Gilbert <address@hidden>

Yes, this took me some thinking about why it got there.
(I only managed to reproduce this case once, even with 'tc')

'LISTEN' message via loadvm_postcopy_handle_listen,
postcopy state is set to LISTENING
sets mis->have_listen_thread
starts 'listen' thread
Errors while 'loading state of instance...' so fails
   qemu_loadvm_state_main in loadvm_handle_cmd_packaged
   fails loadvm_process_command
   fails qemu_loadvm_state_main
   fails in qemu_loadvm_state
       has mis->have_listen_thread
   process_incoming_migration_co
      since ret < 0 fails now rather than leaving it to the
      'listening thread' - which is probably still alive

Dave


> ---
>  migration/migration.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index ad4036f..e024e0a 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -436,9 +436,6 @@ static void process_incoming_migration_co(void *opaque)
>          qemu_thread_join(&mis->colo_incoming_thread);
>      }
>  
> -    qemu_fclose(f);
> -    free_xbzrle_decoded_buf();
> -
>      if (ret < 0) {
>          migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
>                            MIGRATION_STATUS_FAILED);
> @@ -447,6 +444,9 @@ static void process_incoming_migration_co(void *opaque)
>          exit(EXIT_FAILURE);
>      }
>  
> +    qemu_fclose(f);
> +    free_xbzrle_decoded_buf();
> +
>      mis->bh = qemu_bh_new(process_incoming_migration_bh, mis);
>      qemu_bh_schedule(mis->bh);
>  }
> -- 
> 2.9.3
> 
--
Dr. David Alan Gilbert / address@hidden / Manchester, UK