On Mon, Jul 27, 2026 at 04:53:15AM -0400, Michael S. Tsirkin wrote: > 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?
Let's drop it first so it won't block this pull. I can definitely pick it up when ready, but I want to discuss on how to fix first. Which tree to pick it up isn't a huge matter to me, but the review. We found a bunch of randomly introduced VMSTATE flags in the past, I used to remove some, at least my plan is in the future each time we introduce a new one (by request from other modules) we better justify it before landing too easily, in case it needs a revert again. This vhost regression started 11.0 so IIUC we don't need to rush in 11.1. I just noticed I read that patch and didn't notice this, my bad to not have noticed.. For this one specifically.. > > > On Sun, Jul 26, 2026, 5:29 p.m. Michael S. Tsirkin <[email protected]> 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 If there's no real demand to use 2G or more, I suggest we change size to int32_t instead on the user, and revert f6fdd8b2 VMSTATE_VBUFFER_UINT64(). Fabiano and I were looking at rest 20+ possible security related bugs in the past 1-2 weeks, I've some patches to be posted for 11.2 too when we went through all of them. One relevant patch I haven't sent but will do soon: https://gitlab.com/peterx/qemu/-/commit/afd2cb25defdf78a4ab5279cc16634884792c57a We encountered quite a few of possible over-allocation on destionation side by manipulating on-wire length field like this one, I believe Fabiano is looking at how to further limit that from 2G if ever possible per-user. I'm not sure if that idea will fly, but I think anything bigger than 2G definitely is not suggested for now when it's only for a type match not real demand.. I believe VMSTATE_VBUFFER_UINT32 is problematic on its own too, I'll see how to fix that. That one is slightly easier, worst case is we bail out for 2G (hence ignore bit 31, which shouldn't be used in reality for legit users), but I'll think about it. Thanks, > > 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 <[email protected]> > > Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3675 > > Signed-off-by: Michael S. Tsirkin <[email protected]> > > Message-ID: < > > 801b1501ee10241f7ac49a10570d548352b36347.1784890517.git....@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 > > > > > -- Peter Xu
