From: Trieu Huynh <vikingtc4@gmail.com>

mapped_ram_read_header() reads page_size from the migration stream and
stores it in MappedRamHeader, but does not validate that the value is
non-zero before it is later used in parse_ramblock_mapped_ram():

num_pages = length / header.page_size;

If a corrupted or malformed migration stream provides invalid, guest
resumes either with corrupted memory or crashes unexpectedly (eg.
page_size = 0)

Add validation in mapped_ram_read_header() to reject invalid page_size
values early and return an error instead of continuing with an invalid
header.

Steps to reproduce:

Create a migration snapshot with mapped-ram enabled:
(qemu) migrate_set_capability mapped-ram on
(qemu) migrate file:/tmp/qemu-snapshots/snapshot.bin
Modify the snapshot so that MappedRamHeader.page_size becomes diff with
target psize. (0/512/8192/1GB).
Restore the snapshot:
(qemu) migrate_set_capability mapped-ram on
(qemu) migrate_incoming file:/tmp/qemu-snapshots/snapshot.bin

As-is:
* [0]: Floating point exception (core dumped)
* [512/8192]: Silent corruption
* [1GB]: "post load hook failed for: kvm-tpr-opt" (EPERM)
To-be:
* All: qemu-system-x86_64: Migration mapped-ram header has invalid
  page_size [val] (expected 4096)

Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
---
v2:
- Replace == 0 check with != TARGET_PAGE_SIZE, which also catches
  non-zero wrong values and matches the actual invariant of the
  mapped-ram feature (suggested by Peter Xu)
- Include actual and expected values in the error message

 migration/ram.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/migration/ram.c b/migration/ram.c
index 979751f61b..2046f16caa 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -3088,6 +3088,12 @@ static bool mapped_ram_read_header(QEMUFile *file, MappedRamHeader *header,
     }
 
     header->page_size = be64_to_cpu(header->page_size);
+    if (header->page_size != TARGET_PAGE_SIZE) {
+        error_setg(errp, "Migration mapped-ram header has invalid "
+                   "page_size %" PRIu64 " (expected %d)",
+                   header->page_size, TARGET_PAGE_SIZE);
+        return false;
+    }
     header->bitmap_offset = be64_to_cpu(header->bitmap_offset);
     header->pages_offset = be64_to_cpu(header->pages_offset);
 
-- 
2.43.0

Trieu Huynh <vikingtc4@gmail.com> writes:

> From: Trieu Huynh <vikingtc4@gmail.com>
>
> mapped_ram_read_header() reads page_size from the migration stream and
> stores it in MappedRamHeader, but does not validate that the value is
> non-zero before it is later used in parse_ramblock_mapped_ram():
>
> num_pages = length / header.page_size;
>
> If a corrupted or malformed migration stream provides invalid, guest
> resumes either with corrupted memory or crashes unexpectedly (eg.
> page_size = 0)
>
> Add validation in mapped_ram_read_header() to reject invalid page_size
> values early and return an error instead of continuing with an invalid
> header.
>
> Steps to reproduce:
>
> Create a migration snapshot with mapped-ram enabled:
> (qemu) migrate_set_capability mapped-ram on
> (qemu) migrate file:/tmp/qemu-snapshots/snapshot.bin
> Modify the snapshot so that MappedRamHeader.page_size becomes diff with
> target psize. (0/512/8192/1GB).
> Restore the snapshot:
> (qemu) migrate_set_capability mapped-ram on
> (qemu) migrate_incoming file:/tmp/qemu-snapshots/snapshot.bin
>
> As-is:
> * [0]: Floating point exception (core dumped)
> * [512/8192]: Silent corruption
> * [1GB]: "post load hook failed for: kvm-tpr-opt" (EPERM)
> To-be:
> * All: qemu-system-x86_64: Migration mapped-ram header has invalid
>   page_size [val] (expected 4096)
>
> Signed-off-by: Trieu Huynh <vikingtc4@gmail.com>
> ---
> v2:
> - Replace == 0 check with != TARGET_PAGE_SIZE, which also catches
>   non-zero wrong values and matches the actual invariant of the
>   mapped-ram feature (suggested by Peter Xu)
> - Include actual and expected values in the error message
>
>  migration/ram.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/migration/ram.c b/migration/ram.c
> index 979751f61b..2046f16caa 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -3088,6 +3088,12 @@ static bool mapped_ram_read_header(QEMUFile *file, MappedRamHeader *header,
>      }
>  
>      header->page_size = be64_to_cpu(header->page_size);
> +    if (header->page_size != TARGET_PAGE_SIZE) {
> +        error_setg(errp, "Migration mapped-ram header has invalid "
> +                   "page_size %" PRIu64 " (expected %d)",
> +                   header->page_size, TARGET_PAGE_SIZE);
> +        return false;
> +    }
>      header->bitmap_offset = be64_to_cpu(header->bitmap_offset);
>      header->pages_offset = be64_to_cpu(header->pages_offset);

Reviewed-by: Fabiano Rosas <farosas@suse.de>