All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: qemu-devel@nongnu.org
Cc: Peter Maydell <peter.maydell@linaro.org>,
	peterx@redhat.com, Fabiano Rosas <farosas@suse.de>,
	Pierrick Bouvier <pierrick.bouvier@linaro.org>,
	Denis Rastyogin <gerben@altlinux.org>,
	Thomas Huth <thuth@redhat.com>
Subject: [PULL 1/2] migration: Check current_migration in migration_is_running()
Date: Wed, 13 Nov 2024 15:16:30 -0500	[thread overview]
Message-ID: <20241113201631.2920541-2-peterx@redhat.com> (raw)
In-Reply-To: <20241113201631.2920541-1-peterx@redhat.com>

Report shows that commit 34a8892dec broke iotest 055:

https://lore.kernel.org/r/b8806360-a2b6-4608-83a3-db67e264c733@linaro.org

Denis Rastyogin reported more such issue:

https://lore.kernel.org/r/20241107114256.106831-1-gerben@altlinux.org

  In this merge, the migration_is_idle() function was replaced with
  migrate_is_running().  However, the null pointer check for `s` was
  removed, leading to a dereference of `s` when using qemu-system-x86_64
  -hda *.vdi.

When replacing migration_is_idle() with "!migration_is_running()", it was
overlooked that the idle helper also checks for current_migration being
available first.  Sample stack dump:

 migration_is_running
 is_busy
 migrate_add_blocker_modes
 migrate_add_blocker_normal
 vmdk_open
 bdrv_open_driver
 bdrv_open_common
 bdrv_open_inherit
 bdrv_open
 blk_new_open
 blockdev_init
 drive_new
 drive_init_func
 qemu_opts_foreach
 configure_blockdev
 qemu_create_early_backends
 qemu_init
 main

The check would be there if the whole series was applied, but since the
last patches in the previous series rely on some other patches to land
first, we need to recover the behavior of migration_is_idle() first before
that whole set will be merged.

I left migration_is_active / migration_is_device alone, as I don't think
it's possible for them to hit uninitialized current_migration. Also they're
prone to removal soon from VFIO side.

Cc: Peter Maydell <peter.maydell@linaro.org>
Fixes: 34a8892dec ("migration: Drop migration_is_idle()")
Reported-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reported-by: Denis Rastyogin <gerben@altlinux.org>
Tested-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Tested-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
Link: https://lore.kernel.org/r/20241105182725.2393425-1-peterx@redhat.com
[peterx: enhance commit msg]
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 migration/migration.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/migration/migration.c b/migration/migration.c
index aedf7f0751..8c5bd0a75c 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -1117,6 +1117,10 @@ bool migration_is_running(void)
 {
     MigrationState *s = current_migration;
 
+    if (!s) {
+        return false;
+    }
+
     switch (s->state) {
     case MIGRATION_STATUS_ACTIVE:
     case MIGRATION_STATUS_POSTCOPY_ACTIVE:
-- 
2.45.0



  reply	other threads:[~2024-11-13 20:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-13 20:16 [PULL 0/2] Migration 20241113 patches Peter Xu
2024-11-13 20:16 ` Peter Xu [this message]
2024-11-13 20:16 ` [PULL 2/2] migration: fix-possible-int-overflow Peter Xu
2024-11-13 20:40   ` Philippe Mathieu-Daudé
2024-11-13 21:17     ` Peter Xu
2024-11-15 18:52 ` [PULL 0/2] Migration 20241113 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=20241113201631.2920541-2-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=farosas@suse.de \
    --cc=gerben@altlinux.org \
    --cc=peter.maydell@linaro.org \
    --cc=pierrick.bouvier@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.