External email: Use caution opening links or attachments
Avihai Horon <avihaih@nvidia.com> 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 <avihaih@nvidia.com>
---
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 835608cd23..21e8f9d4d4 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -744,7 +744,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);
+ }
}
Reviewed-by: Juan Quintela <quintela@redhat.com>
I mean that the change is right.
But I am not so sure about using qemu_file_* operations outside of the
migration_thread. I *think* that everything else that uses qemu_file_*
functions is operating inside the migration_thread, and this one don't
look like it. Furthermore, a fast look at qemu source, I can't see
anyplace where we setup an error in a vmstate_change.