Re: [PATCH 1/5] migration: Fix possible overflow in vmstate_handle_alloc()

[Fabiano Rosas <farosas@suse.de>]

Peter Xu <peterx@redhat.com> writes:

> Migration incoming side almost always trusted the stream data and allows
> allocation to happen with whatever size received.  With it, malicious
> migration stream can manipulate destination QEMU behavior on g_malloc(), in
> path of vmstate_handle_alloc() on specific VMSD fields.  Fix it by limiting
> all sizes with int32_t positive values (INT_MAX) explicitly.
>
> We have quite a few bug reports recently leveraging this defect. It can be
> reproduced in many ways for (I think) all archs binaries, but the simplest
> reproducer is:
>
>   $ hexdump -C ./vm.img
>   00000000  51 45 56 4d 00 00 00 03  07 80 00 00 00 00 00 00  |QEVM............|
>
>   $ ./qemu-system-x86_64 -incoming file:./vm.img
>   VNC server running on ::1:5900
>   qemu-system-x86_64: GLib: ../glib/gmem.c:106: failed to allocate 18446744071562067968 bytes
>   Aborted                    (core dumped) ./qemu-system-x86_64 -incoming file:./vm.img
>
> We could assert here, but since we have errp right above the stack this
> patch routes the errp over to allow destination QEMU fail gracefully.  This
> means there's no way to DoS coredumpctl as well because we don't generate
> core dumps at all. The output message could also hopefully help triage
> issues when it's not a malicious stream but only wrong image used.
>
> When at this, making sure multiplex also won't overflow.
>
> After patched:
>
>   $ ./qemu-system-x86_64 -incoming file:./vm.img
>   VNC server running on ::1:5900
>   qemu-system-x86_64: load of migration failed: Invalid argument: vmstate_size: VMState field 'name' overflow
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3805
> Reported-by: Feifan Qian <bea1e@proton.me>
> Reported-by: dong ling (@dongling226655)
> Cc: qemu-stable <qemu-stable@nongnu.org>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: Fabiano Rosas <farosas@suse.de>
> Signed-off-by: Peter Xu <peterx@redhat.com>

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