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: 김승중 <[email protected]> > > Cc: Alexandr Moshkov <[email protected]> > > Cc: Michael S. Tsirkin <[email protected]> > > Cc: Fabiano Rosas <[email protected]> > > Fixes: 3a80ff0721 ("vhost: add vmstate for inflight region with inner > > buffer") > > Signed-off-by: Peter Xu <[email protected]> > > --- > > > > 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/[email protected] > > > > 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() > > } > > };
