starred/qemu
1
0
Fork 0
mirror of https://github.com/qemu/qemu.git synced 2026-07-08 17:46:10 +00:00
Code Issues 380 Packages Projects Releases Wiki Activity

migration: Use QAPI_CLONE_MEMBERS in migrate_params_test_apply

Browse Source 
Use QAPI_CLONE_MEMBERS instead of making an assignment. The QAPI
method makes the handling of the TLS strings more intuitive because it
clones them as well.

This also fixes a segfault when a NULL TLS option is accessed as part
of a validation check for another option (e.g. in the zero-copy +
multifd compression case). Details follow:

Currently, after copying s->parameters to the temporary
MigrationParameters object before migrate_params_check(), the
references in temporary object to the TLS options are dropped, either
because:

a) the user set a new option, in which case that's fine as
   s->parameters still holds the reference to the old memory or,

b) the user did not set a new option, in which case keeping the
   references in the temporary object would later cause them to be
   freed along with it, leading to double-free when s->parameters is
   also freed later on.

In this second case, it was overlooked that the TLS options can be
accessed already during migrate_params_check() as part of validation
of another option. Those pointers should not have been cleared.

Using QAPI_CLONE_MEMBERS fixes the issue because the temporary object
is not stealing a reference from s->parameters anymore.

Cc: qemu-stable <qemu-stable@nongnu.org>
Fixes: aed97f0563 ("migration: Normalize tls arguments")
Reported-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Link: https://lore.kernel.org/r/a65a1049-9f19-460a-8e27-a62bb30d2727@maciej.szmigiero.name
Reviewed-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Fabiano Rosas <farosas@suse.de>
Tested-by: Maciej S. Szmigiero <maciej.szmigiero@oracle.com>
Link: https://lore.kernel.org/r/20260414223718.23965-1-farosas@suse.de
Signed-off-by: Peter Xu <peterx@redhat.com>
This commit is contained in:
Fabiano Rosas
2026-04-14 19:37:18 -03:00
committed by Peter Xu
parent 1ae7d9c30d
commit f77a7cec9f
1 changed files with 12 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

26
migration/options.c
View File

@@ -1279,9 +1279,9 @@ bool migrate_params_check(MigrationParameters *params, Error **errp)
static void migrate_params_test_apply(MigrationParameters *params,
MigrationParameters *dest)
{
*dest = migrate_get_current()->parameters;
MigrationState *s = migrate_get_current();
/* TODO use QAPI_CLONE() instead of duplicating it inline */
QAPI_CLONE_MEMBERS(MigrationParameters, dest, &s->parameters);
if (params->has_throttle_trigger_threshold) {
dest->throttle_trigger_threshold = params->throttle_trigger_threshold;
@@ -1300,24 +1300,18 @@ static void migrate_params_test_apply(MigrationParameters *params,
}
if (params->tls_creds) {
qapi_free_StrOrNull(dest->tls_creds);
dest->tls_creds = QAPI_CLONE(StrOrNull, params->tls_creds);
} else {
/* clear the reference, it's owned by s->parameters */
dest->tls_creds = NULL;
}
if (params->tls_hostname) {
qapi_free_StrOrNull(dest->tls_hostname);
dest->tls_hostname = QAPI_CLONE(StrOrNull, params->tls_hostname);
} else {
/* clear the reference, it's owned by s->parameters */
dest->tls_hostname = NULL;
}
if (params->tls_authz) {
qapi_free_StrOrNull(dest->tls_authz);
dest->tls_authz = QAPI_CLONE(StrOrNull, params->tls_authz);
} else {
/* clear the reference, it's owned by s->parameters */
dest->tls_authz = NULL;
}
if (params->has_max_bandwidth) {
@@ -1374,8 +1368,9 @@ static void migrate_params_test_apply(MigrationParameters *params,
}
if (params->has_block_bitmap_mapping) {
dest->has_block_bitmap_mapping = true;
dest->block_bitmap_mapping = params->block_bitmap_mapping;
qapi_free_BitmapMigrationNodeAliasList(dest->block_bitmap_mapping);
dest->block_bitmap_mapping = QAPI_CLONE(BitmapMigrationNodeAliasList,
params->block_bitmap_mapping);
}
if (params->has_x_vcpu_dirty_limit_period) {
@@ -1399,7 +1394,8 @@ static void migrate_params_test_apply(MigrationParameters *params,
}
if (params->has_cpr_exec_command) {
dest->cpr_exec_command = params->cpr_exec_command;
qapi_free_strList(dest->cpr_exec_command);
dest->cpr_exec_command = QAPI_CLONE(strList, params->cpr_exec_command);
}
}
@@ -1555,4 +1551,6 @@ void qmp_migrate_set_parameters(MigrationParameters *params, Error **errp)
}
migrate_tls_opts_free(&tmp);
qapi_free_BitmapMigrationNodeAliasList(tmp.block_bitmap_mapping);
qapi_free_strList(tmp.cpr_exec_command);
}