Re: [PATCH v2] migration: validate page_size in mapped-ram header before use

[Fabiano Rosas <farosas@suse.de>]

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>