Skip to content

Commit 825b96d

Browse files
committed
Merge tag 'migration-20250310-pull-request' of https://gitlab.com/farosas/qemu into staging
Migration pull request - Fix use-after-free in incoming migration - Improve cpr migration blocker for volatile ram - Fix RDMA migration # -----BEGIN PGP SIGNATURE----- # # iQJEBAABCAAuFiEEqhtIsKIjJqWkw2TPx5jcdBvsMZ0FAmfPaCAQHGZhcm9zYXNA # c3VzZS5kZQAKCRDHmNx0G+wxnQy9EADRp/6GaSzoqWgafU8DGM5Q69HyKiZ888DZ # 7qXqJeH3c95nvOnIw2BMhUYX4t8kkAbUcWlr7L8KCjZT/6N/d1/Z5fimqymRkw4x # +8kDyADv5FY0339aMLf3qBbIAQj/gvPvg8H+e+hXfokZqoYgLXZ0eqNAz8MjIcyN # +A+waEBMLNvTgZyTQl2TbCvb+mbRial8u8C9BIoILhn/gNuoMX7lbt0tq41HZwe0 # l3v16jnXlsDvQUXp99bGySomRgkcYqdAt+HWHLje3frT/Ap8dGaUJKlpgJ8DXJiA # fV1reKihJdj37q9GSG8cR02W+ATBesiecufV4TUPNQYQzTdxn3fOMwdc3Pck074D # YAQxFT20OPou+NRxjYoHT/GqFUY36/2qBJpt7TY3ramdklHJhXpRyedK4rppTZNn # pC3lnbpA/LHRmfD1Nh0CRmqZpbV+qW1BWEgMwk4qui46BxYWHxKHFpxAuwlJQmcw # RxY8qPhIXQM03tiTgIddBNDZLoVqRoUP7YpzR7MMa1rz0T5inNFMcNGm72WpKODE # rzpw4ezXO7+D4/QmMq3PoPfhFv3QFnH6jaGj8JkJM378KLvh4fQ0woXtDKFl4Tbq # 1oBZ17WUv6aHr75b+KMyKJNLinvMu5WF5WoRYIt1lNXaqk7I494yvIjtRrimWZIS # Z5Q0tpUmpw== # =yEH0 # -----END PGP SIGNATURE----- # gpg: Signature made Tue 11 Mar 2025 06:30:56 HKT # gpg: using RSA key AA1B48B0A22326A5A4C364CFC798DC741BEC319D # gpg: issuer "[email protected]" # gpg: Good signature from "Fabiano Rosas <[email protected]>" [unknown] # gpg: aka "Fabiano Almeida Rosas <[email protected]>" [unknown] # gpg: WARNING: The key's User ID is not certified with a trusted signature! # gpg: There is no indication that the signature belongs to the owner. # Primary key fingerprint: AA1B 48B0 A223 26A5 A4C3 64CF C798 DC74 1BEC 319D * tag 'migration-20250310-pull-request' of https://gitlab.com/farosas/qemu: migration: Prioritize RDMA in ram_save_target_page() migration: ram block cpr blockers migration: Fix UAF for incoming migration on MigrationState Signed-off-by: Stefan Hajnoczi <[email protected]>
2 parents 1a5f3d2 + baa41af commit 825b96d

File tree

6 files changed

+115
-6
lines changed

6 files changed

+115
-6
lines changed

include/exec/memory.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3203,6 +3203,9 @@ bool ram_block_discard_is_disabled(void);
32033203
*/
32043204
bool ram_block_discard_is_required(void);
32053205

3206+
void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp);
3207+
void ram_block_del_cpr_blocker(RAMBlock *rb);
3208+
32063209
#endif
32073210

32083211
#endif

include/exec/ramblock.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ struct RAMBlock {
3939
/* RCU-enabled, writes protected by the ramlist lock */
4040
QLIST_ENTRY(RAMBlock) next;
4141
QLIST_HEAD(, RAMBlockNotifier) ramblock_notifiers;
42+
Error *cpr_blocker;
4243
int fd;
4344
uint64_t fd_offset;
4445
int guest_memfd;

migration/migration.c

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,27 @@ static void migration_downtime_start(MigrationState *s)
116116
s->downtime_start = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
117117
}
118118

119+
/*
120+
* This is unfortunate: incoming migration actually needs the outgoing
121+
* migration state (MigrationState) to be there too, e.g. to query
122+
* capabilities, parameters, using locks, setup errors, etc.
123+
*
124+
* NOTE: when calling this, making sure current_migration exists and not
125+
* been freed yet! Otherwise trying to access the refcount is already
126+
* an use-after-free itself..
127+
*
128+
* TODO: Move shared part of incoming / outgoing out into separate object.
129+
* Then this is not needed.
130+
*/
131+
static void migrate_incoming_ref_outgoing_state(void)
132+
{
133+
object_ref(migrate_get_current());
134+
}
135+
static void migrate_incoming_unref_outgoing_state(void)
136+
{
137+
object_unref(migrate_get_current());
138+
}
139+
119140
static void migration_downtime_end(MigrationState *s)
120141
{
121142
int64_t now = qemu_clock_get_ms(QEMU_CLOCK_REALTIME);
@@ -863,7 +884,7 @@ process_incoming_migration_co(void *opaque)
863884
* postcopy thread.
864885
*/
865886
trace_process_incoming_migration_co_postcopy_end_main();
866-
return;
887+
goto out;
867888
}
868889
/* Else if something went wrong then just fall out of the normal exit */
869890
}
@@ -879,7 +900,8 @@ process_incoming_migration_co(void *opaque)
879900
}
880901

881902
migration_bh_schedule(process_incoming_migration_bh, mis);
882-
return;
903+
goto out;
904+
883905
fail:
884906
migrate_set_state(&mis->state, MIGRATION_STATUS_ACTIVE,
885907
MIGRATION_STATUS_FAILED);
@@ -896,6 +918,9 @@ process_incoming_migration_co(void *opaque)
896918

897919
exit(EXIT_FAILURE);
898920
}
921+
out:
922+
/* Pairs with the refcount taken in qmp_migrate_incoming() */
923+
migrate_incoming_unref_outgoing_state();
899924
}
900925

901926
/**
@@ -1901,6 +1926,17 @@ void qmp_migrate_incoming(const char *uri, bool has_channels,
19011926
return;
19021927
}
19031928

1929+
/*
1930+
* Making sure MigrationState is available until incoming migration
1931+
* completes.
1932+
*
1933+
* NOTE: QEMU _might_ leak this refcount in some failure paths, but
1934+
* that's OK. This is the minimum change we need to at least making
1935+
* sure success case is clean on the refcount. We can try harder to
1936+
* make it accurate for any kind of failures, but it might be an
1937+
* overkill and doesn't bring us much benefit.
1938+
*/
1939+
migrate_incoming_ref_outgoing_state();
19041940
once = false;
19051941
}
19061942

migration/ram.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1964,6 +1964,11 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
19641964
ram_addr_t offset = ((ram_addr_t)pss->page) << TARGET_PAGE_BITS;
19651965
int res;
19661966

1967+
/* Hand over to RDMA first */
1968+
if (control_save_page(pss, offset, &res)) {
1969+
return res;
1970+
}
1971+
19671972
if (!migrate_multifd()
19681973
|| migrate_zero_page_detection() == ZERO_PAGE_DETECTION_LEGACY) {
19691974
if (save_zero_page(rs, pss, offset)) {
@@ -1976,10 +1981,6 @@ static int ram_save_target_page(RAMState *rs, PageSearchStatus *pss)
19761981
return ram_save_multifd_page(block, offset);
19771982
}
19781983

1979-
if (control_save_page(pss, offset, &res)) {
1980-
return res;
1981-
}
1982-
19831984
return ram_save_page(rs, pss);
19841985
}
19851986

migration/savevm.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3514,12 +3514,14 @@ void vmstate_register_ram(MemoryRegion *mr, DeviceState *dev)
35143514
qemu_ram_set_idstr(mr->ram_block,
35153515
memory_region_name(mr), dev);
35163516
qemu_ram_set_migratable(mr->ram_block);
3517+
ram_block_add_cpr_blocker(mr->ram_block, &error_fatal);
35173518
}
35183519

35193520
void vmstate_unregister_ram(MemoryRegion *mr, DeviceState *dev)
35203521
{
35213522
qemu_ram_unset_idstr(mr->ram_block);
35223523
qemu_ram_unset_migratable(mr->ram_block);
3524+
ram_block_del_cpr_blocker(mr->ram_block);
35233525
}
35243526

35253527
void vmstate_register_ram_global(MemoryRegion *mr)

system/physmem.c

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,10 @@
7171

7272
#include "qemu/pmem.h"
7373

74+
#include "qapi/qapi-types-migration.h"
75+
#include "migration/blocker.h"
7476
#include "migration/cpr.h"
77+
#include "migration/options.h"
7578
#include "migration/vmstate.h"
7679

7780
#include "qemu/range.h"
@@ -1904,6 +1907,14 @@ static void ram_block_add(RAMBlock *new_block, Error **errp)
19041907
qemu_mutex_unlock_ramlist();
19051908
goto out_free;
19061909
}
1910+
1911+
error_setg(&new_block->cpr_blocker,
1912+
"Memory region %s uses guest_memfd, "
1913+
"which is not supported with CPR.",
1914+
memory_region_name(new_block->mr));
1915+
migrate_add_blocker_modes(&new_block->cpr_blocker, errp,
1916+
MIG_MODE_CPR_TRANSFER,
1917+
-1);
19071918
}
19081919

19091920
ram_size = (new_block->offset + new_block->max_length) >> TARGET_PAGE_BITS;
@@ -4095,3 +4106,58 @@ bool ram_block_discard_is_required(void)
40954106
return qatomic_read(&ram_block_discard_required_cnt) ||
40964107
qatomic_read(&ram_block_coordinated_discard_required_cnt);
40974108
}
4109+
4110+
/*
4111+
* Return true if ram is compatible with CPR. Do not exclude rom,
4112+
* because the rom file could change in new QEMU.
4113+
*/
4114+
static bool ram_is_cpr_compatible(RAMBlock *rb)
4115+
{
4116+
MemoryRegion *mr = rb->mr;
4117+
4118+
if (!mr || !memory_region_is_ram(mr)) {
4119+
return true;
4120+
}
4121+
4122+
/* Ram device is remapped in new QEMU */
4123+
if (memory_region_is_ram_device(mr)) {
4124+
return true;
4125+
}
4126+
4127+
/*
4128+
* A file descriptor is passed to new QEMU and remapped, or its backing
4129+
* file is reopened and mapped. It must be shared to avoid COW.
4130+
*/
4131+
if (rb->fd >= 0 && qemu_ram_is_shared(rb)) {
4132+
return true;
4133+
}
4134+
4135+
return false;
4136+
}
4137+
4138+
/*
4139+
* Add a blocker for each volatile ram block. This function should only be
4140+
* called after we know that the block is migratable. Non-migratable blocks
4141+
* are either re-created in new QEMU, or are handled specially, or are covered
4142+
* by a device-level CPR blocker.
4143+
*/
4144+
void ram_block_add_cpr_blocker(RAMBlock *rb, Error **errp)
4145+
{
4146+
assert(qemu_ram_is_migratable(rb));
4147+
4148+
if (ram_is_cpr_compatible(rb)) {
4149+
return;
4150+
}
4151+
4152+
error_setg(&rb->cpr_blocker,
4153+
"Memory region %s is not compatible with CPR. share=on is "
4154+
"required for memory-backend objects, and aux-ram-share=on is "
4155+
"required.", memory_region_name(rb->mr));
4156+
migrate_add_blocker_modes(&rb->cpr_blocker, errp, MIG_MODE_CPR_TRANSFER,
4157+
-1);
4158+
}
4159+
4160+
void ram_block_del_cpr_blocker(RAMBlock *rb)
4161+
{
4162+
migrate_del_blocker(&rb->cpr_blocker);
4163+
}

0 commit comments

Comments
 (0)