qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: peterx@redhat.com
To: qemu-devel@nongnu.org, Peter Maydell <peter.maydell@linaro.org>
Cc: "Fabiano Rosas" <farosas@suse.de>,
	"Prasad Pandit" <ppandit@redhat.com>,
	peterx@redhat.com, "David Hildenbrand" <david@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>
Subject: [PULL 02/10] migration: Fix error handling after dup in file migration
Date: Sun, 17 Mar 2024 16:57:55 -0400	[thread overview]
Message-ID: <20240317205803.361163-3-peterx@redhat.com> (raw)
In-Reply-To: <20240317205803.361163-1-peterx@redhat.com>

From: Fabiano Rosas <farosas@suse.de>

The file migration code was allowing a possible -1 from a failed call
to dup() to propagate into the new QIOFileChannel::fd before checking
for validity. Coverity doesn't like that, possibly due to the the
lseek(-1, ...) call that would ensue before returning from the channel
creation routine.

Use the newly introduced qio_channel_file_dupfd() to properly check
the return of dup() before proceeding.

Fixes: CID 1539961
Fixes: CID 1539965
Fixes: CID 1539960
Fixes: 2dd7ee7a51 ("migration/multifd: Add incoming QIOChannelFile support")
Fixes: decdc76772 ("migration/multifd: Add mapped-ram support to fd: URI")
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Reviewed-by: "Daniel P. Berrangé" <berrange@redhat.com>
Link: https://lore.kernel.org/r/20240311233335.17299-3-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/fd.c   |  9 ++++-----
 migration/file.c | 14 +++++++-------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/migration/fd.c b/migration/fd.c
index d4ae72d132..4e2a63a73d 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -80,6 +80,7 @@ static gboolean fd_accept_incoming_migration(QIOChannel *ioc,
 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;
@@ -103,15 +104,13 @@ void fd_start_incoming_migration(const char *fdname, Error **errp)
         int channels = migrate_multifd_channels();
 
         while (channels--) {
-            ioc = QIO_CHANNEL(qio_channel_file_new_fd(dup(fd)));
-
-            if (QIO_CHANNEL_FILE(ioc)->fd == -1) {
-                error_setg(errp, "Failed to duplicate fd %d", fd);
+            fioc = qio_channel_file_new_dupfd(fd, errp);
+            if (!fioc) {
                 return;
             }
 
             qio_channel_set_name(ioc, "migration-fd-incoming");
-            qio_channel_add_watch_full(ioc, G_IO_IN,
+            qio_channel_add_watch_full(QIO_CHANNEL(fioc), G_IO_IN,
                                        fd_accept_incoming_migration,
                                        NULL, NULL,
                                        g_main_context_get_thread_default());
diff --git a/migration/file.c b/migration/file.c
index b0b963e0ce..e56c5eb0a5 100644
--- a/migration/file.c
+++ b/migration/file.c
@@ -58,12 +58,13 @@ bool file_send_channel_create(gpointer opaque, Error **errp)
     int fd = fd_args_get_fd();
 
     if (fd && fd != -1) {
-        ioc = qio_channel_file_new_fd(dup(fd));
+        ioc = qio_channel_file_new_dupfd(fd, errp);
     } else {
         ioc = qio_channel_file_new_path(outgoing_args.fname, flags, 0, errp);
-        if (!ioc) {
-            goto out;
-        }
+    }
+
+    if (!ioc) {
+        goto out;
     }
 
     multifd_channel_connect(opaque, QIO_CHANNEL(ioc));
@@ -147,10 +148,9 @@ void file_start_incoming_migration(FileMigrationArgs *file_args, Error **errp)
                                    NULL, NULL,
                                    g_main_context_get_thread_default());
 
-        fioc = qio_channel_file_new_fd(dup(fioc->fd));
+        fioc = qio_channel_file_new_dupfd(fioc->fd, errp);
 
-        if (!fioc || fioc->fd == -1) {
-            error_setg(errp, "Error creating migration incoming channel");
+        if (!fioc) {
             break;
         }
     } while (++i < channels);
-- 
2.44.0



  parent reply	other threads:[~2024-03-17 20:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-17 20:57 [PULL 00/10] Migration 20240317 patches peterx
2024-03-17 20:57 ` [PULL 01/10] io: Introduce qio_channel_file_new_dupfd peterx
2024-03-17 20:57 ` peterx [this message]
2024-03-17 20:57 ` [PULL 03/10] physmem: Expose tlb_reset_dirty_range_all() peterx
2024-03-17 20:57 ` [PULL 04/10] physmem: Factor cpu_physical_memory_dirty_bits_cleared() out peterx
2024-03-17 20:57 ` [PULL 05/10] physmem: Fix migration dirty bitmap coherency with TCG memory access peterx
2024-03-17 20:57 ` [PULL 06/10] migration: Skip only empty block devices peterx
2024-03-17 20:58 ` [PULL 07/10] migration: cpr-reboot documentation peterx
2024-03-17 20:58 ` [PULL 08/10] migration: Fix iocs leaks during file and fd migration peterx
2024-03-17 20:58 ` [PULL 09/10] migration/multifd: Ensure we're not given a socket for file migration peterx
2024-03-17 20:58 ` [PULL 10/10] migration/multifd: Duplicate the fd for the outgoing_args peterx
2024-03-19 10:23 ` [PULL 00/10] Migration 20240317 patches Peter Maydell

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240317205803.361163-3-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=berrange@redhat.com \
    --cc=david@redhat.com \
    --cc=farosas@suse.de \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=ppandit@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).