| Message ID | 20170412135312.1686-2-lvivier@redhat.com |
|---|---|
| State | New |
| Headers | show |
* 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
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: Juan Quintela <quintela@redhat.com>
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); }
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(-)