From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: peterx@redhat.com, "Fabiano Rosas" <farosas@suse.de>,
"Igor Mammedov" <imammedo@redhat.com>,
"Juraj Marcin" <jmarcin@redhat.com>,
"Michael S . Tsirkin" <mst@redhat.com>,
"Dr . David Alan Gilbert" <dave@treblig.org>,
"Cédric Le Goater" <clg@redhat.com>,
"Markus Armbruster" <armbru@redhat.com>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Daniel P . Berrangé" <berrange@redhat.com>,
"Alex Williamson" <alex.williamson@redhat.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Peter Maydell" <peter.maydell@linaro.org>
Subject: [PATCH 4/4] migration: Reset current_migration properly
Date: Thu, 24 Oct 2024 12:56:27 -0400 [thread overview]
Message-ID: <20241024165627.1372621-5-peterx@redhat.com> (raw)
In-Reply-To: <20241024165627.1372621-1-peterx@redhat.com>
current_migration is never reset, even if the migration object is freed
already. It means anyone references that can trigger UAF and it'll be hard
to debug.
Properly clear the pointer now, so far the only way to do is via
finalize() as we know there's only one instance of it, meanwhile QEMU won't
know who holds the refcount, so it can't reset the variable manually but
only in finalize().
To make it more readable, also initialize the variable in the
instance_init() so it's very well paired at least.
Signed-off-by: Peter Xu <peterx@redhat.com>
---
migration/migration.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
diff --git a/migration/migration.c b/migration/migration.c
index 1b5285af95..74812ca785 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -233,9 +233,11 @@ static int migration_stop_vm(MigrationState *s, RunState state)
void migration_object_init(void)
{
- /* This can only be called once. */
- assert(!current_migration);
- current_migration = MIGRATION_OBJ(object_new(TYPE_MIGRATION));
+ /* This creates the singleton migration object */
+ object_new(TYPE_MIGRATION);
+
+ /* This should be set now when initialize the singleton object */
+ assert(current_migration);
/*
* Init the migrate incoming object as well no matter whether
@@ -3886,12 +3888,27 @@ static void migration_instance_finalize(Object *obj)
qemu_sem_destroy(&ms->rp_state.rp_pong_acks);
qemu_sem_destroy(&ms->postcopy_qemufile_src_sem);
error_free(ms->error);
+
+ /*
+ * We know we only have one intance of migration, and when reaching
+ * here it means migration object is gone. Clear the global reference
+ * to reflect that.
+ */
+ current_migration = NULL;
}
static void migration_instance_init(Object *obj)
{
MigrationState *ms = MIGRATION_OBJ(obj);
+ /*
+ * There can only be one migration object globally. Keep a record of
+ * the pointer in current_migration, which will be reset after the
+ * object finalize().
+ */
+ assert(!current_migration);
+ current_migration = ms;
+
ms->state = MIGRATION_STATUS_NONE;
ms->mbps = -1;
ms->pages_per_second = -1;
--
2.45.0
next prev parent reply other threads:[~2024-10-24 16:58 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-10-24 16:56 [PATCH 0/4] QOM: Singleton interface Peter Xu
2024-10-24 16:56 ` [PATCH 1/4] qom: TYPE_SINGLETON interface Peter Xu
2024-10-24 20:02 ` Philippe Mathieu-Daudé
2024-10-24 20:53 ` Peter Xu
2024-10-25 15:11 ` Philippe Mathieu-Daudé
2024-10-25 16:21 ` Peter Xu
2024-10-25 8:07 ` Markus Armbruster
2024-10-25 15:17 ` Peter Xu
2024-10-25 9:51 ` Daniel P. Berrangé
2024-10-25 16:17 ` Peter Xu
2024-10-25 16:22 ` Daniel P. Berrangé
2024-10-25 22:10 ` Peter Xu
2024-10-29 0:01 ` Peter Xu
2024-10-25 16:37 ` Peter Xu
2024-10-24 16:56 ` [PATCH 2/4] x86/iommu: Make x86-iommu a singleton object Peter Xu
2024-10-25 9:25 ` Markus Armbruster
2024-10-25 21:55 ` Peter Xu
2024-10-25 22:13 ` Peter Xu
2024-11-07 11:12 ` Markus Armbruster
2024-11-07 15:29 ` Peter Xu
2024-11-08 8:50 ` Markus Armbruster
2024-10-29 10:47 ` Daniel P. Berrangé
2024-10-29 14:32 ` Peter Xu
2024-10-24 16:56 ` [PATCH 3/4] migration: Make migration object " Peter Xu
2024-10-24 19:20 ` Fabiano Rosas
2024-10-24 16:56 ` Peter Xu [this message]
2024-10-24 19:34 ` [PATCH 4/4] migration: Reset current_migration properly Fabiano Rosas
2024-10-24 20:15 ` Peter Xu
2024-10-24 20:51 ` Fabiano Rosas
2024-10-25 7:38 ` [PATCH 0/4] QOM: Singleton interface Markus Armbruster
2024-10-25 15:01 ` Peter Xu
2024-10-29 10:42 ` Daniel P. Berrangé
2024-10-29 14:45 ` Peter Xu
2024-10-29 16:04 ` Daniel P. Berrangé
2024-10-29 17:05 ` Peter Xu
2024-10-29 17:17 ` Daniel P. Berrangé
2024-12-11 8:19 ` Markus Armbruster
2024-12-11 22:10 ` Peter Xu
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=20241024165627.1372621-5-peterx@redhat.com \
--to=peterx@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=armbru@redhat.com \
--cc=berrange@redhat.com \
--cc=clg@redhat.com \
--cc=dave@treblig.org \
--cc=eduardo@habkost.net \
--cc=farosas@suse.de \
--cc=imammedo@redhat.com \
--cc=jmarcin@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--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).