* Avihai Horon ([email protected]) wrote:
> As part of its error flow, vfio_vmstate_change() accesses
> MigrationState->to_dst_file without any checks. This can cause a NULL
> pointer dereference if the error flow is taken and
> MigrationState->to_dst_file is not set.
>
> For example, this can happen if VM is started or stopped not during
> migration and vfio_vmstate_change() error flow is taken, as
> MigrationState->to_dst_file is not set at that time.
>
> Fix it by checking that MigrationState->to_dst_file is set before using
> it.
>
> Fixes: 02a7e71b1e5b ("vfio: Add VM state change handler to know state of VM")
> Signed-off-by: Avihai Horon <[email protected]>
> Reviewed-by: Juan Quintela <[email protected]>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <[email protected]>
It might be worth posting this patch separately since it's a simple fix
and should go in sooner.
Dave
> ---
> hw/vfio/migration.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index e1413ac90c..09fe7c1de2 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -743,7 +743,9 @@ static void vfio_vmstate_change(void *opaque, bool
> running, RunState state)
> */
> error_report("%s: Failed to set device state 0x%x", vbasedev->name,
> (migration->device_state & mask) | value);
> - qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> + if (migrate_get_current()->to_dst_file) {
> + qemu_file_set_error(migrate_get_current()->to_dst_file, ret);
> + }
> }
> vbasedev->migration->vm_running = running;
> trace_vfio_vmstate_change(vbasedev->name, running, RunState_str(state),
> --
> 2.26.3
>
--
Dr. David Alan Gilbert / [email protected] / Manchester, UK