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

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

[Peter Xu]

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.

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()
     }
 };
-- 
2.54.0

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

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> 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.
>
> 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),                        \
> -}

Why don't you just leave this macro around, put a "do not use" comment
above it and remove the type check for this one instance? You're already
checking against INT32_MAX during pre_load.

We could even lift that check up into vmstate_pre_load and reject
there globally, don't even touch the virtio code.


/rant
... what's even the point of having per-type variants of VMSTATE macros
that exist just to type-check the extra offsets (.num_offset,
.size_offset as opposed to .offset)?

For instance, look at vmstate_n_elems casting opaque data to 32 and
sub-32bit size and assigning to an int! What do we gain from that? It's
circular reasoning that does nothing aside from bothering the person
writing the vmstate.

Right? Maybe I'm missing something, it's the end of the day already.

> -
>  #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()
>      }
>  };

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

[Michael S. Tsirkin]

On Tue, Jul 28, 2026 at 11:39:42AM -0400, 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.
> 
> 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.


I agree.

> ---
>  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()
>      }
>  };
> -- 
> 2.54.0

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

[Alexandr Moshkov]

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;
}

>
> 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()
>       }
>   };

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

[Michael S. Tsirkin]

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()
> >       }
> >   };

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

[Peter Xu]

On Tue, Jul 28, 2026 at 05:57:56PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
> 
> > 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.
> >
> > 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),                        \
> > -}
> 
> Why don't you just leave this macro around, put a "do not use" comment
> above it and remove the type check for this one instance? You're already
> checking against INT32_MAX during pre_load.

What's the benefit of keeping this global macro if we only allow one user
to use it?

Commenting to say "don't use" is not guaranteeing anything, at least we
should also rename the macro to some weird name to make people notice.
Keeping the macro name like this OTOH suggests abuse.  But if we think it
should only be used in 1 place, fixing the one place might be better?

> 
> We could even lift that check up into vmstate_pre_load and reject
> there globally, don't even touch the virtio code.

Personally, I don't like the idea leaking VBUFFER impl details into
vmstate_pre_load() which is so far only a wrapper, especially only for this
one instance.. which we do not suggest future users to use.

If we go this route, I'd rather merge Michael's version to support u64,
even if we don't need a u64 size.  But I really don't want to introduce yet
another VMS flag just for this... we'll have no real use if we have noticed
this problem when the vhost inflight patch was reviewed.  It will be a
uint32_t or int32_t already.  I just can't come up with some users need
size >2G.

The recent AI reports just make such feeling stronger: we just used the
reason "max 2G, should be fine" when AI reports that unlimited size
allocation problem, now we will need to wait for another AI report which
says "it's not 2G anymore, 1<<64-1 that is", if we don't come up with a
proper limit for VMSD field allocations.

> 
> 
> /rant
> ... what's even the point of having per-type variants of VMSTATE macros
> that exist just to type-check the extra offsets (.num_offset,
> .size_offset as opposed to .offset)?
> 
> For instance, look at vmstate_n_elems casting opaque data to 32 and
> sub-32bit size and assigning to an int! What do we gain from that? It's
> circular reasoning that does nothing aside from bothering the person
> writing the vmstate.
> 
> Right? Maybe I'm missing something, it's the end of the day already.

I can also only guess, which is.. I believe Michael was right, the initial
vmstate code was simply broken where it should have considered the type of
size fields of all kinds, but forgot, and it just worked because nobody
needed u64 for a size field.

Said that, I still want to see if we can avoid introducing that at all.  To
me, I still prefer this patch (after fixing pre_save()..).  But let me know
if I didn't convince any of you.. we can discuss.

Thanks,

> 
> > -
> >  #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()
> >      }
> >  };
> 

-- 
Peter Xu

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

[Peter Xu]

On Wed, Jul 29, 2026 at 04:49:13AM -0400, Michael S. Tsirkin wrote:
> 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;
> > }

Thanks for the testing and report, Alexandr.  Obviously I only kept in mind
of the cross-binary case..  I'll see if I'll respin with the fix or
something different.

> 
> 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?

I explained in my reply to Fabiano:

https://lore.kernel.org/qemu-devel/amoHbVsgzkYclCt2@x1.local/

Thanks,

-- 
Peter Xu

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

[Michael S. Tsirkin]

On Wed, Jul 29, 2026 at 10:00:13AM -0400, Peter Xu wrote:
> On Tue, Jul 28, 2026 at 05:57:56PM -0300, Fabiano Rosas wrote:
> > Peter Xu <peterx@redhat.com> writes:
> > 
> > > 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.
> > >
> > > 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),                        \
> > > -}
> > 
> > Why don't you just leave this macro around, put a "do not use" comment
> > above it and remove the type check for this one instance? You're already
> > checking against INT32_MAX during pre_load.
> 
> What's the benefit of keeping this global macro if we only allow one user
> to use it?
> 
> Commenting to say "don't use" is not guaranteeing anything, at least we
> should also rename the macro to some weird name to make people notice.
> Keeping the macro name like this OTOH suggests abuse.  But if we think it
> should only be used in 1 place, fixing the one place might be better?
> 
> > 
> > We could even lift that check up into vmstate_pre_load and reject
> > there globally, don't even touch the virtio code.
> 
> Personally, I don't like the idea leaking VBUFFER impl details into
> vmstate_pre_load() which is so far only a wrapper, especially only for this
> one instance.. which we do not suggest future users to use.
> 
> If we go this route, I'd rather merge Michael's version to support u64,
> even if we don't need a u64 size.  But I really don't want to introduce yet
> another VMS flag just for this... we'll have no real use if we have noticed
> this problem when the vhost inflight patch was reviewed.  It will be a
> uint32_t or int32_t already.  I just can't come up with some users need
> size >2G.


It's not that size needs to be >2G. But it might be nice for some code
to keep it as u64 for its own reasons to avoid issues like overruns
during math ops.

> 
> The recent AI reports just make such feeling stronger: we just used the
> reason "max 2G, should be fine" when AI reports that unlimited size
> allocation problem, now we will need to wait for another AI report which
> says "it's not 2G anymore, 1<<64-1 that is", if we don't come up with a
> proper limit for VMSD field allocations.
> 
> > 
> > 
> > /rant
> > ... what's even the point of having per-type variants of VMSTATE macros
> > that exist just to type-check the extra offsets (.num_offset,
> > .size_offset as opposed to .offset)?
> > 
> > For instance, look at vmstate_n_elems casting opaque data to 32 and
> > sub-32bit size and assigning to an int! What do we gain from that? It's
> > circular reasoning that does nothing aside from bothering the person
> > writing the vmstate.
> > 
> > Right? Maybe I'm missing something, it's the end of the day already.
> 
> I can also only guess, which is.. I believe Michael was right, the initial
> vmstate code was simply broken where it should have considered the type of
> size fields of all kinds, but forgot, and it just worked because nobody
> needed u64 for a size field.
> 
> Said that, I still want to see if we can avoid introducing that at all.  To
> me, I still prefer this patch (after fixing pre_save()..).  But let me know
> if I didn't convince any of you.. we can discuss.
> 
> Thanks,


The issue is that 2G is not a sane limit either.

So what we really want is a macro that supplies
a size limit. That would be an improvement.

> > 
> > > -
> > >  #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()
> > >      }
> > >  };
> > 
> 
> -- 
> Peter Xu

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

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Tue, Jul 28, 2026 at 05:57:56PM -0300, Fabiano Rosas wrote:
>> Peter Xu <peterx@redhat.com> writes:
>> 
>> > 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.
>> >
>> > 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),                        \
>> > -}
>> 
>> Why don't you just leave this macro around, put a "do not use" comment
>> above it and remove the type check for this one instance? You're already
>> checking against INT32_MAX during pre_load.
>
> What's the benefit of keeping this global macro if we only allow one user
> to use it?
>
> Commenting to say "don't use" is not guaranteeing anything, at least we
> should also rename the macro to some weird name to make people notice.
> Keeping the macro name like this OTOH suggests abuse.  But if we think it
> should only be used in 1 place, fixing the one place might be better?
>

I was thinking of fixing this on the vmstate size and not changing the
virtio code. But yes, it's better to fix properly.

>> 
>> We could even lift that check up into vmstate_pre_load and reject
>> there globally, don't even touch the virtio code.
>
> Personally, I don't like the idea leaking VBUFFER impl details into
> vmstate_pre_load() which is so far only a wrapper, especially only for this
> one instance.. which we do not suggest future users to use.
>

Hm, but isn't it better to have a global cap anyway? So we don't need
every device code to change with similar checks.

> If we go this route, I'd rather merge Michael's version to support u64,
> even if we don't need a u64 size.  But I really don't want to introduce yet
> another VMS flag just for this... we'll have no real use if we have noticed
> this problem when the vhost inflight patch was reviewed.  It will be a
> uint32_t or int32_t already.  I just can't come up with some users need
> size >2G.
>

I agree with making all u64. I think we can actually remove all the
extra VMS_VARRAY_* and VBUFFER_* flags and instead doa single type-check
of "int <= 64bit". Give me a couple of hours and I will post an RFC.

> The recent AI reports just make such feeling stronger: we just used the
> reason "max 2G, should be fine" when AI reports that unlimited size
> allocation problem, now we will need to wait for another AI report which
> says "it's not 2G anymore, 1<<64-1 that is", if we don't come up with a
> proper limit for VMSD field allocations.
>
>> 
>> 
>> /rant
>> ... what's even the point of having per-type variants of VMSTATE macros
>> that exist just to type-check the extra offsets (.num_offset,
>> .size_offset as opposed to .offset)?
>> 
>> For instance, look at vmstate_n_elems casting opaque data to 32 and
>> sub-32bit size and assigning to an int! What do we gain from that? It's
>> circular reasoning that does nothing aside from bothering the person
>> writing the vmstate.
>> 
>> Right? Maybe I'm missing something, it's the end of the day already.
>
> I can also only guess, which is.. I believe Michael was right, the initial
> vmstate code was simply broken where it should have considered the type of
> size fields of all kinds, but forgot, and it just worked because nobody
> needed u64 for a size field.
>
> Said that, I still want to see if we can avoid introducing that at all.  To
> me, I still prefer this patch (after fixing pre_save()..).  But let me know
> if I didn't convince any of you.. we can discuss.
>
> Thanks,
>
>> 
>> > -
>> >  #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()
>> >      }
>> >  };
>> 

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

[Peter Xu]

On Wed, Jul 29, 2026 at 11:27:44AM -0300, Fabiano Rosas wrote:
> Hm, but isn't it better to have a global cap anyway? So we don't need
> every device code to change with similar checks.
> 
> > If we go this route, I'd rather merge Michael's version to support u64,
> > even if we don't need a u64 size.  But I really don't want to introduce yet
> > another VMS flag just for this... we'll have no real use if we have noticed
> > this problem when the vhost inflight patch was reviewed.  It will be a
> > uint32_t or int32_t already.  I just can't come up with some users need
> > size >2G.
> >
> 
> I agree with making all u64. I think we can actually remove all the
> extra VMS_VARRAY_* and VBUFFER_* flags and instead doa single type-check
> of "int <= 64bit". Give me a couple of hours and I will post an RFC.

I confess I don't yet get what is the global cap you mentioned.. but sure,
I'll wait and read the RFC first.

-- 
Peter Xu

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

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Wed, Jul 29, 2026 at 11:27:44AM -0300, Fabiano Rosas wrote:
>> Hm, but isn't it better to have a global cap anyway? So we don't need
>> every device code to change with similar checks.
>> 
>> > If we go this route, I'd rather merge Michael's version to support u64,
>> > even if we don't need a u64 size.  But I really don't want to introduce yet
>> > another VMS flag just for this... we'll have no real use if we have noticed
>> > this problem when the vhost inflight patch was reviewed.  It will be a
>> > uint32_t or int32_t already.  I just can't come up with some users need
>> > size >2G.
>> >
>> 
>> I agree with making all u64. I think we can actually remove all the
>> extra VMS_VARRAY_* and VBUFFER_* flags and instead doa single type-check
>> of "int <= 64bit". Give me a couple of hours and I will post an RFC.
>
> I confess I don't yet get what is the global cap you mentioned.. but sure,
> I'll wait and read the RFC first.

I was thinking the vmstate code could define a limit to the size (of
anything) and always enforce it. The device code can then use some
custom macros (not yet existent) to limit even further.