[PULL 04/10] migration/multifd: Fix leaks of TLS error objects

Fabiano Rosas posted 10 patches 2 weeks, 6 days ago
Maintainers: "Daniel P. Berrangé" <berrange@redhat.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, Laurent Vivier <lvivier@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Mark Kanda <mark.kanda@oracle.com>, Ben Chaney <bchaney@akamai.com>, "Dr. David Alan Gilbert" <dave@treblig.org>
[PULL 04/10] migration/multifd: Fix leaks of TLS error objects
Posted by Fabiano Rosas 2 weeks, 6 days ago
The code currently ignores errors from multifd threads that happen
after a first error has already been propagated. Make sure the
subsequent errors are freed appopriately.

This fixes a leak of the TLS session->werr when the certificate
validation fails after multifd threads are already running. The first
writes on the threads will fail deep into the gnutls stack.

No need to check if(err) because the callers are all under a similar
check.

Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>
Link: https://lore.kernel.org/qemu-devel/20260311213418.16951-5-farosas@suse.de
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
 migration/multifd.c | 27 ++++++++++++---------------
 1 file changed, 12 insertions(+), 15 deletions(-)

diff --git a/migration/multifd.c b/migration/multifd.c
index 8b9ed84805..035cb70f7b 100644
--- a/migration/multifd.c
+++ b/migration/multifd.c
@@ -412,28 +412,25 @@ bool multifd_send(MultiFDSendData **send_data)
 /* Multifd send side hit an error; remember it and prepare to quit */
 static void multifd_send_error_propagate(Error *err)
 {
+    MigrationState *s = migrate_get_current();
+
     /*
-     * We don't want to exit each threads twice.  Depending on where
-     * we get the error, or if there are two independent errors in two
-     * threads at the same time, we can end calling this function
-     * twice.
+     * There may be independent errors in each thread. Propagate the
+     * first and free the subsequent ones.
      */
     if (qatomic_xchg(&multifd_send_state->exiting, 1)) {
+        error_free(err);
         return;
     }
 
-    if (err) {
-        MigrationState *s = migrate_get_current();
+    migrate_error_propagate(s, err);
 
-        migrate_error_propagate(s, err);
-
-        if (s->state == MIGRATION_STATUS_SETUP ||
-            s->state == MIGRATION_STATUS_PRE_SWITCHOVER ||
-            s->state == MIGRATION_STATUS_DEVICE ||
-            s->state == MIGRATION_STATUS_ACTIVE) {
-            migrate_set_state(&s->state, s->state,
-                              MIGRATION_STATUS_FAILING);
-        }
+    if (s->state == MIGRATION_STATUS_SETUP ||
+        s->state == MIGRATION_STATUS_PRE_SWITCHOVER ||
+        s->state == MIGRATION_STATUS_DEVICE ||
+        s->state == MIGRATION_STATUS_ACTIVE) {
+        migrate_set_state(&s->state, s->state,
+                          MIGRATION_STATUS_FAILING);
     }
 }
 
-- 
2.51.0