[PATCH] migration: Fix iocs leaks during file and fd migration

[Fabiano Rosas]

The memory for the io channels is being leaked in three different ways
during file migration:

1) if the offset check fails we never drop the ioc reference;

2) if multifd is not enabled, we allocate an extra channel for no
   reason;

3) if multifd is enabled but channel creation fails when calling
   dup(), we leave the previous channels around along with the glib
   polling;

Fix all issues by restructuring the code to first allocate the
channels and only register the watches when all channels have been
created.

The file and fd migrations can share this code because both are backed
by the file migration infrastructure.

Fixes: 2dd7ee7a51 ("migration/multifd: Add incoming QIOChannelFile support")
Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI")
Reported-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
---
based-on: https://gitlab.com/peterx/migration-stable
CI run: https://gitlab.com/farosas/qemu/-/pipelines/1211909831
---
 migration/fd.c   | 28 +++-------------------
 migration/file.c | 62 +++++++++++++++++++++++++++++++-----------------
 migration/file.h |  3 +++
 3 files changed, 46 insertions(+), 47 deletions(-)

diff --git a/migration/fd.c b/migration/fd.c
index 4e2a63a73d..b74a3eb8c8 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -18,6 +18,7 @@
 #include "qapi/error.h"
 #include "channel.h"
 #include "fd.h"
+#include "file.h"
 #include "migration.h"
 #include "monitor/monitor.h"
 #include "io/channel-file.h"
@@ -68,19 +69,9 @@ void fd_start_outgoing_migration(MigrationState *s, const 
char *fdname, Error **
     object_unref(OBJECT(ioc));
 }
 
-static gboolean fd_accept_incoming_migration(QIOChannel *ioc,
-                                             GIOCondition condition,
-                                             gpointer opaque)
-{
-    migration_channel_process_incoming(ioc);
-    object_unref(OBJECT(ioc));
-    return G_SOURCE_REMOVE;
-}
-
 void fd_start_incoming_migration(const char *fdname, Error **errp)
 {
     QIOChannel *ioc;
-    QIOChannelFile *fioc;
     int fd = monitor_fd_param(monitor_cur(), fdname, errp);
     if (fd == -1) {
         return;
@@ -96,24 +87,11 @@ void fd_start_incoming_migration(const char *fdname, Error 
**errp)
 
     qio_channel_set_name(ioc, "migration-fd-incoming");
     qio_channel_add_watch_full(ioc, G_IO_IN,
-                               fd_accept_incoming_migration,
+                               file_accept_incoming_migration,
                                NULL, NULL,
                                g_main_context_get_thread_default());
 
     if (migrate_multifd()) {
-        int channels = migrate_multifd_channels();
-
-        while (channels--) {
-            fioc = qio_channel_file_new_dupfd(fd, errp);
-            if (!fioc) {
-                return;
-            }
-
-            qio_channel_set_name(ioc, "migration-fd-incoming");
-            qio_channel_add_watch_full(QIO_CHANNEL(fioc), G_IO_IN,
-                                       fd_accept_incoming_migration,
-                                       NULL, NULL,
-                                       g_main_context_get_thread_default());
-        }
+        file_recv_channels_create(fd, errp);
     }
 }
diff --git a/migration/file.c b/migration/file.c
index e56c5eb0a5..bb264115eb 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -106,22 +106,48 @@ void file_start_outgoing_migration(MigrationState *s,
     migration_channel_connect(s, ioc, NULL, NULL);
 }
 
-static gboolean file_accept_incoming_migration(QIOChannel *ioc,
-                                               GIOCondition condition,
-                                               gpointer opaque)
+gboolean file_accept_incoming_migration(QIOChannel *ioc, GIOCondition 
condition,
+                                        gpointer opaque)
 {
     migration_channel_process_incoming(ioc);
     object_unref(OBJECT(ioc));
     return G_SOURCE_REMOVE;
 }
 
+void file_recv_channels_create(int fd, Error **errp)
+{
+    int i, channels = migrate_multifd_channels();
+    g_autofree QIOChannelFile **fiocs = g_new0(QIOChannelFile *, channels);
+
+    for (i = 0; i < channels; i++) {
+        QIOChannelFile *fioc = qio_channel_file_new_dupfd(fd, errp);
+
+        if (!fioc) {
+            while (i) {
+                object_unref(fiocs[--i]);
+            }
+            return;
+        }
+
+        fiocs[i] = fioc;
+    }
+
+    for (i = 0; i < channels; i++) {
+        QIOChannelFile *fioc = fiocs[i];
+
+        qio_channel_set_name(QIO_CHANNEL(fioc), "migration-file-incoming");
+        qio_channel_add_watch_full(QIO_CHANNEL(fioc), G_IO_IN,
+                                   file_accept_incoming_migration,
+                                   NULL, NULL,
+                                   g_main_context_get_thread_default());
+    }
+}
+
 void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp)
 {
     g_autofree char *filename = g_strdup(file_args->filename);
     QIOChannelFile *fioc = NULL;
     uint64_t offset = file_args->offset;
-    int channels = 1;
-    int i = 0;
 
     trace_migration_file_incoming(filename);
 
@@ -132,28 +158,20 @@ void file_start_incoming_migration(FileMigrationArgs 
*file_args, Error **errp)
 
     if (offset &&
         qio_channel_io_seek(QIO_CHANNEL(fioc), offset, SEEK_SET, errp) < 0) {
+        object_unref(OBJECT(fioc));
         return;
     }
 
+    qio_channel_set_name(QIO_CHANNEL(fioc), "migration-file-incoming");
+    qio_channel_add_watch_full(QIO_CHANNEL(fioc), G_IO_IN,
+                               file_accept_incoming_migration,
+                               NULL, NULL,
+                               g_main_context_get_thread_default());
+
+
     if (migrate_multifd()) {
-        channels += migrate_multifd_channels();
+        file_recv_channels_create(fioc->fd, errp);
     }
-
-    do {
-        QIOChannel *ioc = QIO_CHANNEL(fioc);
-
-        qio_channel_set_name(ioc, "migration-file-incoming");
-        qio_channel_add_watch_full(ioc, G_IO_IN,
-                                   file_accept_incoming_migration,
-                                   NULL, NULL,
-                                   g_main_context_get_thread_default());
-
-        fioc = qio_channel_file_new_dupfd(fioc->fd, errp);
-
-        if (!fioc) {
-            break;
-        }
-    } while (++i < channels);
 }
 
 int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
diff --git a/migration/file.h b/migration/file.h
index 9f71e87f74..940122db0f 100644
--- a/migration/file.h
+++ b/migration/file.h
@@ -20,6 +20,9 @@ void file_start_outgoing_migration(MigrationState *s,
 int file_parse_offset(char *filespec, uint64_t *offsetp, Error **errp);
 void file_cleanup_outgoing_migration(void);
 bool file_send_channel_create(gpointer opaque, Error **errp);
+void file_recv_channels_create(int fd, Error **errp);
+gboolean file_accept_incoming_migration(QIOChannel *ioc, GIOCondition 
condition,
+                                        gpointer opaque);
 int file_write_ramblock_iov(QIOChannel *ioc, const struct iovec *iov,
                             int niov, RAMBlock *block, Error **errp);
 int multifd_file_recv_data(MultiFDRecvParams *p, Error **errp);
-- 
2.35.3