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 <lvivier@redhat.com>
---
 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

* Laurent Vivier (lvivier@redhat.com) 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 <lvivier@redhat.com>

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

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 / dgilbert@redhat.com / Manchester, UK