| Message ID | 20240328140252.16756-3-avihaih@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | migration: Two migration bug fixes | expand |
On 3/28/24 15:02, Avihai Horon wrote: > There are several places where postcopy_start() fails without setting > errp. This can cause a null pointer de-reference, as in case of error, > the caller of postcopy_start() copies/prints the error set in errp. > > Fix it by setting errp in all of postcopy_start() error paths. > > Fixes: 908927db28ea ("migration: Update error description whenever migration fails") > Signed-off-by: Avihai Horon <avihaih@nvidia.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > migration/migration.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/migration/migration.c b/migration/migration.c > index b73ae3a72c4..86bf76e9258 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2510,6 +2510,8 @@ static int postcopy_start(MigrationState *ms, Error **errp) > migration_wait_main_channel(ms); > if (postcopy_preempt_establish_channel(ms)) { > migrate_set_state(&ms->state, ms->state, MIGRATION_STATUS_FAILED); > + error_setg(errp, "%s: Failed to establish preempt channel", > + __func__); > return -1; > } > } > @@ -2525,17 +2527,22 @@ static int postcopy_start(MigrationState *ms, Error **errp) > > ret = migration_stop_vm(ms, RUN_STATE_FINISH_MIGRATE); > if (ret < 0) { > + error_setg_errno(errp, -ret, "%s: Failed to stop the VM", __func__); > goto fail; > } > > ret = migration_maybe_pause(ms, &cur_state, > MIGRATION_STATUS_POSTCOPY_ACTIVE); > if (ret < 0) { > + error_setg_errno(errp, -ret, "%s: Failed in migration_maybe_pause()", > + __func__); > goto fail; > } > > ret = bdrv_inactivate_all(); > if (ret < 0) { > + error_setg_errno(errp, -ret, "%s: Failed in bdrv_inactivate_all()", > + __func__); > goto fail; > } > restart_block = true; > @@ -2612,6 +2619,7 @@ static int postcopy_start(MigrationState *ms, Error **errp) > > /* Now send that blob */ > if (qemu_savevm_send_packaged(ms->to_dst_file, bioc->data, bioc->usage)) { > + error_setg(errp, "%s: Failed to send packaged data", __func__); > goto fail_closefb; > } > qemu_fclose(fb);
On Thu, Mar 28, 2024 at 04:02:52PM +0200, Avihai Horon wrote: > There are several places where postcopy_start() fails without setting > errp. This can cause a null pointer de-reference, as in case of error, > the caller of postcopy_start() copies/prints the error set in errp. > > Fix it by setting errp in all of postcopy_start() error paths. > > Fixes: 908927db28ea ("migration: Update error description whenever migration fails") I think this should need: Cc: qemu-stable <qemu-stable@nongnu.org> > Signed-off-by: Avihai Horon <avihaih@nvidia.com> Reviewed-by: Peter Xu <peterx@redhat.com>
diff --git a/migration/migration.c b/migration/migration.c index b73ae3a72c4..86bf76e9258 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2510,6 +2510,8 @@ static int postcopy_start(MigrationState *ms, Error **errp) migration_wait_main_channel(ms); if (postcopy_preempt_establish_channel(ms)) { migrate_set_state(&ms->state, ms->state, MIGRATION_STATUS_FAILED); + error_setg(errp, "%s: Failed to establish preempt channel", + __func__); return -1; } } @@ -2525,17 +2527,22 @@ static int postcopy_start(MigrationState *ms, Error **errp) ret = migration_stop_vm(ms, RUN_STATE_FINISH_MIGRATE); if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed to stop the VM", __func__); goto fail; } ret = migration_maybe_pause(ms, &cur_state, MIGRATION_STATUS_POSTCOPY_ACTIVE); if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed in migration_maybe_pause()", + __func__); goto fail; } ret = bdrv_inactivate_all(); if (ret < 0) { + error_setg_errno(errp, -ret, "%s: Failed in bdrv_inactivate_all()", + __func__); goto fail; } restart_block = true; @@ -2612,6 +2619,7 @@ static int postcopy_start(MigrationState *ms, Error **errp) /* Now send that blob */ if (qemu_savevm_send_packaged(ms->to_dst_file, bioc->data, bioc->usage)) { + error_setg(errp, "%s: Failed to send packaged data", __func__); goto fail_closefb; } qemu_fclose(fb);
There are several places where postcopy_start() fails without setting errp. This can cause a null pointer de-reference, as in case of error, the caller of postcopy_start() copies/prints the error set in errp. Fix it by setting errp in all of postcopy_start() error paths. Fixes: 908927db28ea ("migration: Update error description whenever migration fails") Signed-off-by: Avihai Horon <avihaih@nvidia.com> --- migration/migration.c | 8 ++++++++ 1 file changed, 8 insertions(+)