POSTCOPY_NOTIFY_INBOUND_END handlers will remove userfault fds
from the postcopy_remote_fds array which could be still in
use by the fault thread. Let's stop the thread before
notification to avoid possible accessing wrong memory.

Fixes: 46343570c06e ("vhost+postcopy: Wire up POSTCOPY_END notify")
Cc: qemu-stable@nongnu.org
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 migration/postcopy-ram.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
index 853d8b32ca..e5c02a32c5 100644
--- a/migration/postcopy-ram.c
+++ b/migration/postcopy-ram.c
@@ -533,6 +533,12 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
     if (mis->have_fault_thread) {
         Error *local_err = NULL;
 
+        /* Let the fault thread quit */
+        atomic_set(&mis->fault_thread_quit, 1);
+        postcopy_fault_thread_notify(mis);
+        trace_postcopy_ram_incoming_cleanup_join();
+        qemu_thread_join(&mis->fault_thread);
+
         if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_END, &local_err)) {
             error_report_err(local_err);
             return -1;
@@ -541,11 +547,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
         if (qemu_ram_foreach_migratable_block(cleanup_range, mis)) {
             return -1;
         }
-        /* Let the fault thread quit */
-        atomic_set(&mis->fault_thread_quit, 1);
-        postcopy_fault_thread_notify(mis);
-        trace_postcopy_ram_incoming_cleanup_join();
-        qemu_thread_join(&mis->fault_thread);
 
         trace_postcopy_ram_incoming_cleanup_closeuf();
         close(mis->userfault_fd);
-- 
2.17.1

* Ilya Maximets (i.maximets@samsung.com) wrote:
> POSTCOPY_NOTIFY_INBOUND_END handlers will remove userfault fds
> from the postcopy_remote_fds array which could be still in
> use by the fault thread. Let's stop the thread before
> notification to avoid possible accessing wrong memory.

OK I think; since this is already in the cleanup we shouldn't
be getting faults anyway at that point.


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

> Fixes: 46343570c06e ("vhost+postcopy: Wire up POSTCOPY_END notify")
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  migration/postcopy-ram.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index 853d8b32ca..e5c02a32c5 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -533,6 +533,12 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>      if (mis->have_fault_thread) {
>          Error *local_err = NULL;
>  
> +        /* Let the fault thread quit */
> +        atomic_set(&mis->fault_thread_quit, 1);
> +        postcopy_fault_thread_notify(mis);
> +        trace_postcopy_ram_incoming_cleanup_join();
> +        qemu_thread_join(&mis->fault_thread);
> +
>          if (postcopy_notify(POSTCOPY_NOTIFY_INBOUND_END, &local_err)) {
>              error_report_err(local_err);
>              return -1;
> @@ -541,11 +547,6 @@ int postcopy_ram_incoming_cleanup(MigrationIncomingState *mis)
>          if (qemu_ram_foreach_migratable_block(cleanup_range, mis)) {
>              return -1;
>          }
> -        /* Let the fault thread quit */
> -        atomic_set(&mis->fault_thread_quit, 1);
> -        postcopy_fault_thread_notify(mis);
> -        trace_postcopy_ram_incoming_cleanup_join();
> -        qemu_thread_join(&mis->fault_thread);
>  
>          trace_postcopy_ram_incoming_cleanup_closeuf();
>          close(mis->userfault_fd);
> -- 
> 2.17.1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK