* Peter Xu (peterx@redhat.com) wrote:
> test_migrate_start() will release the MigrateStart structure that passed
> in, however that's not super clear to the caller because after the call
> returned the pointer can still be referenced by the callers. It can easily
> be a source of use-after-free.
>
> Let's pass in a double pointer of that, then we can safely clear the
> pointer for the caller after the struct is released.
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
> tests/qtest/migration-test.c | 27 +++++++++++++++------------
> 1 file changed, 15 insertions(+), 12 deletions(-)
>
> diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
> index 09a9ce4401..67f0601988 100644
> --- a/tests/qtest/migration-test.c
> +++ b/tests/qtest/migration-test.c
> @@ -496,7 +496,7 @@ static void migrate_start_destroy(MigrateStart *args)
> }
>
> static int test_migrate_start(QTestState **from, QTestState **to,
> - const char *uri, MigrateStart *args)
> + const char *uri, MigrateStart **pargs)
> {
> g_autofree gchar *arch_source = NULL;
> g_autofree gchar *arch_target = NULL;
> @@ -508,6 +508,7 @@ static int test_migrate_start(QTestState **from, QTestState **to,
> g_autofree char *shmem_path = NULL;
> const char *arch = qtest_get_arch();
> const char *machine_opts = NULL;
> + MigrateStart *args = *pargs;
> const char *memory_size;
> int ret = 0;
>
> @@ -622,6 +623,8 @@ static int test_migrate_start(QTestState **from, QTestState **to,
>
> out:
> migrate_start_destroy(args);
> + /* This tells the caller that this structure is gone */
> + *pargs = NULL;
> return ret;
> }
>
> @@ -668,7 +671,7 @@ static int migrate_postcopy_prepare(QTestState **from_ptr,
> bool postcopy_preempt = args->postcopy_preempt;
> QTestState *from, *to;
>
> - if (test_migrate_start(&from, &to, uri, args)) {
> + if (test_migrate_start(&from, &to, uri, &args)) {
> return -1;
> }
>
> @@ -822,7 +825,7 @@ static void test_baddest(void)
>
> args->hide_stderr = true;
>
> - if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", args)) {
> + if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
> return;
> }
> migrate_qmp(from, "tcp:127.0.0.1:0", "{}");
> @@ -838,7 +841,7 @@ static void test_precopy_unix_common(bool dirty_ring)
>
> args->use_dirty_ring = dirty_ring;
>
> - if (test_migrate_start(&from, &to, uri, args)) {
> + if (test_migrate_start(&from, &to, uri, &args)) {
> return;
> }
>
> @@ -926,7 +929,7 @@ static void test_xbzrle(const char *uri)
> MigrateStart *args = migrate_start_new();
> QTestState *from, *to;
>
> - if (test_migrate_start(&from, &to, uri, args)) {
> + if (test_migrate_start(&from, &to, uri, &args)) {
> return;
> }
>
> @@ -980,7 +983,7 @@ static void test_precopy_tcp(void)
> g_autofree char *uri = NULL;
> QTestState *from, *to;
>
> - if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", args)) {
> + if (test_migrate_start(&from, &to, "tcp:127.0.0.1:0", &args)) {
> return;
> }
>
> @@ -1025,7 +1028,7 @@ static void test_migrate_fd_proto(void)
> QDict *rsp;
> const char *error_desc;
>
> - if (test_migrate_start(&from, &to, "defer", args)) {
> + if (test_migrate_start(&from, &to, "defer", &args)) {
> return;
> }
>
> @@ -1105,7 +1108,7 @@ static void do_test_validate_uuid(MigrateStart *args, bool should_fail)
> g_autofree char *uri = g_strdup_printf("unix:%s/migsocket", tmpfs);
> QTestState *from, *to;
>
> - if (test_migrate_start(&from, &to, uri, args)) {
> + if (test_migrate_start(&from, &to, uri, &args)) {
> return;
> }
>
> @@ -1197,7 +1200,7 @@ static void test_migrate_auto_converge(void)
> */
> const int64_t expected_threshold = max_bandwidth * downtime_limit / 1000;
>
> - if (test_migrate_start(&from, &to, uri, args)) {
> + if (test_migrate_start(&from, &to, uri, &args)) {
> return;
> }
>
> @@ -1266,7 +1269,7 @@ static void test_multifd_tcp(const char *method)
> QDict *rsp;
> g_autofree char *uri = NULL;
>
> - if (test_migrate_start(&from, &to, "defer", args)) {
> + if (test_migrate_start(&from, &to, "defer", &args)) {
> return;
> }
>
> @@ -1352,7 +1355,7 @@ static void test_multifd_tcp_cancel(void)
>
> args->hide_stderr = true;
>
> - if (test_migrate_start(&from, &to, "defer", args)) {
> + if (test_migrate_start(&from, &to, "defer", &args)) {
> return;
> }
>
> @@ -1391,7 +1394,7 @@ static void test_multifd_tcp_cancel(void)
> args = migrate_start_new();
> args->only_target = true;
>
> - if (test_migrate_start(&from, &to2, "defer", args)) {
> + if (test_migrate_start(&from, &to2, "defer", &args)) {
> return;
> }
>
> --
> 2.32.0
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK