Re: [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD

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

On Wed, Jul 29, 2026 at 12:25:05PM +0500, Alexandr Moshkov wrote:
> 
> On 7/28/26 20:39, Peter Xu wrote:
> > It was overlooked that VMSTATE_VBUFFER_UINT64() won't really work with an
> > uint64_t, as vmstate core only treats the size as 32bits, and maximum
> > INT32_MAX (see vmstate_size()).
> > 
> > Considering that we do not need real 64bits for the size, stick with the 2G
> > limit, converting the size field into 32bits.
> > 
> > Since we can't touch the wire protocol on migration from an old QEMU, we
> > can't directly modify the type of size to uint32_t.  Instead, we need to
> > introduce a temporary variable for this extremely rare issue __size_32bits
> > to be used only for VMSTATE_VBUFFER_UINT32().  Document it and name it
> > weird enough so people won't get confused on having two size variables.
> > 
> > Remove VMSTATE_VBUFFER_UINT64() altogether, because it was never going to
> > be used right.  It means QEMU will only support 2G max for VMS_VBUFFER.
> > 
> > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3675
> > Reported-by: 김승중 <seungjung0711@gmail.com>
> > Cc: Alexandr Moshkov <dtalexundeer@yandex-team.ru>
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Fabiano Rosas <farosas@suse.de>
> > Fixes: 3a80ff0721 ("vhost: add vmstate for inflight region with inner buffer")
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> > 
> > PS1: I only did smoke test as I'm not fluent with vhost inflight feature.
> > Please kindly try it out if possible. In general, migrations from older
> > QEMU should work even after applied.  One can also treat this as partly-RFC
> > from that.
> 
> It looks like inflight migration is broken:
> 
> qemu-system-x86_64: Missing section footer for
> 0000:00:02.0:00.0:00.0/vhost-user-blk
> migrate_error error=load of migration failed: Invalid argument: Section
> footer error, section_id: 53
> qemu-system-x86_64: load of migration failed: Invalid argument: Section
> footer error, section_id: 53
> 
> As far as I understand this happen because __size_32bits not initialized on
> source - it's never set. So vmstate_size() reads 0, and zero-lenght buffer
> is written into migration stream.
> The destination then allocates the correct buffer but reads 0 bytes from the
> stream, leaving unconsumed data and causing the section footer check to
> fail.
> 
> It can be fixed with adding pre_save (or pre_save_errp) to
> vmstate_vhost_inflight_region_buffer that initialize __size_32bits from
> size:
> 
> static int vhost_inflight_buffer_pre_save(void *opaque)
> {
>     struct vhost_inflight *inflight = opaque;
>     /* Only used in VMSTATE_VBUFFER_UINT32() */
>     inflight->__size_32bits = inflight->size;
>     return 0;
> }

At which point I ask whether open coding all this mess
is so much better.


Way I look at it, vmstate machinery had an API of storing size in u64
that it failed to implement correctly. Why not fix it?



> > 
> > PS2: Michael, we have just discussed what we should define as CVE for
> > migration, and this one shouldn't fall into CVE category, please refer to:
> > 
> > https://lore.kernel.org/r/20260721131457.3062767-1-farosas@suse.de
> > 
> > So I didn't yet attach CVE tag.  Please correct if I'm wrong, thanks.
> > ---
> >   include/hw/virtio/vhost.h   |  5 +++++
> >   include/migration/vmstate.h | 10 ----------
> >   hw/virtio/vhost.c           | 19 ++++++++++++++-----
> >   3 files changed, 19 insertions(+), 15 deletions(-)
> > 
> > diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
> > index 684bafcaad..1d1cc24c04 100644
> > --- a/include/hw/virtio/vhost.h
> > +++ b/include/hw/virtio/vhost.h
> > @@ -17,6 +17,11 @@ struct vhost_inflight {
> >       int fd;
> >       void *addr;
> >       uint64_t size;
> > +    /*
> > +     * This is a temporary variable only used during migration loading to
> > +     * satisfy VMSTATE_VBUFFER_UINT32() typing. Please use @size otherwise.
> > +     */
> > +    uint32_t __size_32bits;
> >       uint64_t offset;
> >       uint16_t queue_size;
> >   };
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 1b7f295417..a349b2d84a 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -782,16 +782,6 @@ extern const VMStateInfo vmstate_info_g_byte_array;
> >       .offset       = offsetof(_state, _field),                        \
> >   }
> > -#define VMSTATE_VBUFFER_UINT64(_field, _state, _version, _test, _field_size) { \
> > -    .name         = (stringify(_field)),                             \
> > -    .version_id   = (_version),                                      \
> > -    .field_exists = (_test),                                         \
> > -    .size_offset  = vmstate_offset_value(_state, _field_size, uint64_t),\
> > -    .info         = &vmstate_info_buffer,                            \
> > -    .flags        = VMS_VBUFFER | VMS_POINTER,                       \
> > -    .offset       = offsetof(_state, _field),                        \
> > -}
> > -
> >   #define VMSTATE_VBUFFER_ALLOC_UINT32(_field, _state, _version,       \
> >                                        _test, _field_size) {           \
> >       .name         = (stringify(_field)),                             \
> > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> > index af41841b52..e7c570d0f5 100644
> > --- a/hw/virtio/vhost.c
> > +++ b/hw/virtio/vhost.c
> > @@ -2022,15 +2022,24 @@ void vhost_get_features_ex(struct vhost_dev *hdev,
> >   static bool vhost_inflight_buffer_pre_load(void *opaque, Error **errp)
> >   {
> >       struct vhost_inflight *inflight = opaque;
> > -
> >       int fd = -1;
> > -    void *addr = qemu_memfd_alloc("vhost-inflight", inflight->size,
> > -                                  F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
> > -                                  &fd, errp);
> > +    void *addr;
> > +
> > +    if (inflight->size > INT32_MAX) {
> > +        error_setg(errp, "inflight size '%"PRIu64"' exceeds "
> > +                   "migration limit '%"PRIu32"'", inflight->size, INT32_MAX);
> > +        return false;
> > +    }
> > +
> > +    addr = qemu_memfd_alloc("vhost-inflight", inflight->size,
> > +                            F_SEAL_GROW | F_SEAL_SHRINK | F_SEAL_SEAL,
> > +                            &fd, errp);
> >       if (!addr) {
> >           return false;
> >       }
> > +    /* Only used in VMSTATE_VBUFFER_UINT32() */
> > +    inflight->__size_32bits = inflight->size;
> >       inflight->offset = 0;
> >       inflight->addr = addr;
> >       inflight->fd = fd;
> > @@ -2042,7 +2051,7 @@ const VMStateDescription vmstate_vhost_inflight_region_buffer = {
> >       .name = "vhost-inflight-region/buffer",
> >       .pre_load_errp = vhost_inflight_buffer_pre_load,
> >       .fields = (const VMStateField[]) {
> > -        VMSTATE_VBUFFER_UINT64(addr, struct vhost_inflight, 0, NULL, size),
> > +        VMSTATE_VBUFFER_UINT32(addr, struct vhost_inflight, 0, NULL, __size_32bits),
> >           VMSTATE_END_OF_LIST()
> >       }
> >   };