Re: [PULL 17/30] vmstate: fix type confusion in vmstate_size() for VMSTATE_VBUFFER_UINT64

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Jul 26, 2026 at 08:31:57PM -0400, Peter Xu wrote:
> This patch is migration only change and hasn't been reviewed.  Give us a few
> days to review it?
> Can't do it now because it is on cellphone and I just bathed my son.
> 
> Please hold off merging.
> 
> Thanks.

OK.  I'll drop this and you will merge it through the migration tree
as appropriate?

> On Sun, Jul 26, 2026, 5:29 p.m. Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     vhost user currently saves the inflight buffer to the migration stream
>     using VMSTATE_VBUFFER_UINT64. The size is controlled by the vhost-user
>     backend.
> 
>     But the implementation of that is broken if size is >2G: it stores the
>     buffer size in a uint64_t field, but vmstate_size() always reads the
>     size field as int32_t regardless of the macro used. This, in turn,
>     causes negative or truncated lengths on load, leading to undersized
>     allocations and down the road out-of-bounds buffer access.
> 
>     There's no practical reason to support such large sizes, so it's enough
>     to validate: read the field as uint64_t when VMS_VBUFFER_UINT64 is set,
>     reject negative or oversized values, and propagate errors to
>     vmstate_load_vmsd().
> 
>     Note: the large value is coming from the backend, not guest, so this
>     shouldn't be considered a security issue. The CVE was assigned before
>     the qemu security policy was updated to exclude this class of bugs.
> 
>     Fixes: CVE-2026-6426
>     Fixes: f6fdd8b2bd ("vmstate: introduce VMSTATE_VBUFFER_UINT64")
>     Cc: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
>     Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3675
>     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>     Message-ID: <
>     801b1501ee10241f7ac49a10570d548352b36347.1784890517.git.mst@redhat.com>
>     ---
>      include/migration/vmstate.h |  5 ++++-
>      migration/vmstate.c         | 31 ++++++++++++++++++++++++++++---
>      2 files changed, 32 insertions(+), 4 deletions(-)
> 
>     diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
>     index 1b7f295417..e7095cd977 100644
>     --- a/include/migration/vmstate.h
>     +++ b/include/migration/vmstate.h
>     @@ -168,6 +168,9 @@ enum VMStateFlags {
>           */
>          VMS_ARRAY_OF_POINTER_AUTO_ALLOC = 0x10000,
> 
>     +    /* Use a uint64_t size field for VMS_VBUFFER instead of int32_t. */
>     +    VMS_VBUFFER_UINT64              = 0x40000,
>     +
>          /* Marker for end of list */
>          VMS_END                         = 0x20000,
>      };
>     @@ -788,7 +791,7 @@ extern const VMStateInfo vmstate_info_g_byte_array;
>          .field_exists = (_test),                                         \
>          .size_offset  = vmstate_offset_value(_state, _field_size, uint64_t),\
>          .info         = &vmstate_info_buffer,                            \
>     -    .flags        = VMS_VBUFFER | VMS_POINTER,                       \
>     +    .flags        = VMS_VBUFFER | VMS_VBUFFER_UINT64 | VMS_POINTER,  \
>          .offset       = offsetof(_state, _field),                        \
>      }
> 
>     diff --git a/migration/vmstate.c b/migration/vmstate.c
>     index 50ebe37845..d7f03a9f5b 100644
>     --- a/migration/vmstate.c
>     +++ b/migration/vmstate.c
>     @@ -103,10 +103,24 @@ static int vmstate_size(void *opaque, const
>     VMStateField *field)
>          int size;
> 
>          if (field->flags & VMS_VBUFFER) {
>     -        size = *(int32_t *)(opaque + field->size_offset);
>     -        if (field->flags & VMS_MULTIPLY) {
>     -            size *= field->size;
>     +        uint64_t usize64;
>     +
>     +        if (field->flags & VMS_VBUFFER_UINT64) {
>     +            usize64 = *(uint64_t *)(opaque + field->size_offset);
>     +        } else {
>     +            int32_t ssize32 = *(int32_t *)(opaque + field->size_offset);
>     +            if (ssize32 < 0) {
>     +                return -1;
>     +            }
>     +            usize64 = ssize32;
>              }
>     +        if (field->flags & VMS_MULTIPLY) {
>     +            usize64 *= field->size;
>     +        }
>     +        if (usize64 > INT_MAX) {
>     +            return -1;
>     +        }
>     +        size = usize64;
>          } else if (field->flags & VMS_ARRAY_OF_POINTER) {
>              /*
>               * For an array of pointer, the each element is always size of a
>     @@ -337,6 +351,11 @@ bool vmstate_load_vmsd(QEMUFile *f, const
>     VMStateDescription *vmsd,
>                  void *first_elem = opaque + field->offset;
>                  int i, n_elems = vmstate_n_elems(opaque, field);
>                  int size = vmstate_size(opaque, field);
>     +            if (size < 0) {
>     +                error_setg(errp, "VMState field '%s': invalid size",
>     +                           field->name);
>     +                return false;
>     +            }
> 
>                  vmstate_handle_alloc(first_elem, field, opaque);
>                  if (field->flags & VMS_POINTER) {
>     @@ -661,6 +680,12 @@ static bool vmstate_save_vmsd_v(QEMUFile *f, const
>     VMStateDescription *vmsd,
>                  bool use_dynamic_array =
>                      field->flags & VMS_ARRAY_OF_POINTER_AUTO_ALLOC;
> 
>     +            if (size < 0) {
>     +                error_setg(errp, "VMState field '%s': invalid size",
>     +                           field->name);
>     +                ok = false;
>     +                goto out;
>     +            }
>                  trace_vmstate_save_state_loop(vmsd->name, field->name,
>     n_elems);
>                  if (field->flags & VMS_POINTER) {
>                      first_elem = *(void **)first_elem;
>     --
>     MST
> 
>