On Thu, Jul 22, 2021 at 01:58:38PM -0400, Peter Xu wrote:
> Accessing from_dst_file is potentially racy in current code base like below:
>
> if (s->from_dst_file)
> do_something(s->from_dst_file);
>
> Because from_dst_file can be reset right after the check in another
> thread (rp_thread). One example is migrate_fd_cancel().
>
> Use the same qemu_file_lock to protect it too, just like to_dst_file.
>
> When it's safe to access without lock, comment it.
>
> There's one special reference in migration_thread() that can be replaced by
> the newly introduced rp_thread_created flag.
>
> Reported-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
(Dave should have helped fixing this which I appreciated a lot, but just make
it be together with the record..)
Below needs to be squashed into the patch:
diff --git a/migration/migration.c b/migration/migration.c
index a50330016c..041b8451a6 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2696,7 +2696,7 @@ static void
migration_release_from_dst_file(MigrationState *ms)
{
QEMUFile *file;
- WITH_QEMU_LOCK_GUARD(&s->qemu_file_lock) {
+ WITH_QEMU_LOCK_GUARD(&ms->qemu_file_lock) {
/*
* Reset the from_dst_file pointer first before releasing it, as we
* can't block within lock section
--
Peter Xu
