Message ID | 20230811150836.2895-4-farosas@suse.de |
---|---|
State | New |
Headers | show |
Series | Fix segfault on migration return path | expand |
On Fri, Aug 11, 2023 at 12:08:29PM -0300, Fabiano Rosas wrote: > diff --git a/migration/migration.c b/migration/migration.c > index 0067c927fa..85c171f32c 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2057,11 +2057,10 @@ static int await_return_path_close_on_source(MigrationState *ms) > * need to cause it to exit. shutdown(2), if we have it, will > * cause it to unblock if it's stuck waiting for the destination. > */ > - if (qemu_file_get_error(ms->to_dst_file)) { > - WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) { > - if (ms->rp_state.from_dst_file) { > - qemu_file_shutdown(ms->rp_state.from_dst_file); > - } > + WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) { > + if (ms->to_dst_file && ms->rp_state.from_dst_file && > + qemu_file_get_error(ms->to_dst_file)) { > + qemu_file_shutdown(ms->rp_state.from_dst_file); > } > } Squash into previous one?
diff --git a/migration/migration.c b/migration/migration.c index 0067c927fa..85c171f32c 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2057,11 +2057,10 @@ static int await_return_path_close_on_source(MigrationState *ms) * need to cause it to exit. shutdown(2), if we have it, will * cause it to unblock if it's stuck waiting for the destination. */ - if (qemu_file_get_error(ms->to_dst_file)) { - WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) { - if (ms->rp_state.from_dst_file) { - qemu_file_shutdown(ms->rp_state.from_dst_file); - } + WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) { + if (ms->to_dst_file && ms->rp_state.from_dst_file && + qemu_file_get_error(ms->to_dst_file)) { + qemu_file_shutdown(ms->rp_state.from_dst_file); } }
Checking ms->to_dst_file for errors when cleaning up the return path could race with migrate_fd_cleanup() which clears the pointer. Since migrate_fd_cleanup() is reachable via qmp_migrate(), which is issued by the user, it is safer if we take the lock when reading ms->to_dst_file. Signed-off-by: Fabiano Rosas <farosas@suse.de> --- migration/migration.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)