include/migration/vmstate.h | 255 ++++++++++------------------- migration/savevm.c | 10 +- migration/vmstate.c | 50 ++++-- rust/bindings/migration-sys/lib.rs | 15 +- rust/migration/src/vmstate.rs | 30 +++- rust/tests/tests/vmstate_tests.rs | 30 ++-- 6 files changed, 177 insertions(+), 213 deletions(-)
Hi, this is basically what I ranted about in: https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de I'm replacing the per-integer-size type checks with a single "int that fits in 32bit" check. This allows several lines of duplicated code to be removed. I haven't changed the macro names in the device code yet. If this series gets positive feedback then I'll send per-subsystem patches doing that. CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 Also tested: - migration-test --full --thorough - x86_64 compat run forwards and backwards for previous 3 QEMU releases - s390x compat run forwards and backwards for previous 2 QEMU releases - ppc64 compat run forwards and backwards for previous QEMU release - migration-test smoke ASAN/UBSAN run Fabiano Rosas (4): migration: Remove VMSTATE_ARRAY_INT32_UNSAFE migration: Introduce VMStateOffset migration: Remove redundant flags migration: Remove duplicate vmstate macros include/migration/vmstate.h | 255 ++++++++++------------------- migration/savevm.c | 10 +- migration/vmstate.c | 50 ++++-- rust/bindings/migration-sys/lib.rs | 15 +- rust/migration/src/vmstate.rs | 30 +++- rust/tests/tests/vmstate_tests.rs | 30 ++-- 6 files changed, 177 insertions(+), 213 deletions(-) -- 2.53.0
On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote: > Hi, this is basically what I ranted about in: > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de > > I'm replacing the per-integer-size type checks with a single "int that > fits in 32bit" check. This allows several lines of duplicated code to > be removed. > > I haven't changed the macro names in the device code yet. If this > series gets positive feedback then I'll send per-subsystem patches > doing that. > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 > Also tested: > - migration-test --full --thorough > - x86_64 compat run forwards and backwards for previous 3 QEMU releases > - s390x compat run forwards and backwards for previous 2 QEMU releases > - ppc64 compat run forwards and backwards for previous QEMU release > - migration-test smoke ASAN/UBSAN run > > Fabiano Rosas (4): > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE > migration: Introduce VMStateOffset > migration: Remove redundant flags > migration: Remove duplicate vmstate macros Nice work! I think I was only looking at VBUFFER side and I thought it was fine sticking with 32bit even signed or not, not a huge deal. But cleaning up VARRAY whole thing together looks definitely an improvement. I definitely like your version here. I assume with your series I can drop both of my patches here, right? [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc() https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com (I'll still respin with the rest) [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com The only missing piece would be an multiply overflow check in vmstate_handle_alloc(), if you could add that check too while rewritting that in patch 1 then I think it'll cover all. Vladimir's ask in the separate email makes sense: I wonder if we can also do one step further and merge VBUFFER into VARRAY. The other trivial thing is, while looking, I found one trivial macro VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether. -- Peter Xu
Peter Xu <peterx@redhat.com> writes: > On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote: >> Hi, this is basically what I ranted about in: >> https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de >> >> I'm replacing the per-integer-size type checks with a single "int that >> fits in 32bit" check. This allows several lines of duplicated code to >> be removed. >> >> I haven't changed the macro names in the device code yet. If this >> series gets positive feedback then I'll send per-subsystem patches >> doing that. >> >> CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 >> Also tested: >> - migration-test --full --thorough >> - x86_64 compat run forwards and backwards for previous 3 QEMU releases >> - s390x compat run forwards and backwards for previous 2 QEMU releases >> - ppc64 compat run forwards and backwards for previous QEMU release >> - migration-test smoke ASAN/UBSAN run >> >> Fabiano Rosas (4): >> migration: Remove VMSTATE_ARRAY_INT32_UNSAFE >> migration: Introduce VMStateOffset >> migration: Remove redundant flags >> migration: Remove duplicate vmstate macros > > Nice work! > > I think I was only looking at VBUFFER side and I thought it was fine > sticking with 32bit even signed or not, not a huge deal. But cleaning up > VARRAY whole thing together looks definitely an improvement. I definitely > like your version here. > > I assume with your series I can drop both of my patches here, right? > > [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc() > https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com > (I'll still respin with the rest) > > [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD > https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com > > The only missing piece would be an multiply overflow check in > vmstate_handle_alloc(), if you could add that check too while rewritting > that in patch 1 then I think it'll cover all. > > Vladimir's ask in the separate email makes sense: I wonder if we can also > do one step further and merge VBUFFER into VARRAY. > > The other trivial thing is, while looking, I found one trivial macro > VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether. I can look at it all. Just a heads-up, I'll be off for a couple of weeks starting tomorrow.
On Thu, Jul 30, 2026 at 12:37:13PM -0300, Fabiano Rosas wrote: > I can look at it all. Just a heads-up, I'll be off for a couple of weeks > starting tomorrow. Thanks. I think it's fine we resolve this after your back, as long as Michael is ok with it from vhost side. It's not real CVE, so I hope it's OK. -- Peter Xu
On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote: > On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote: > > Hi, this is basically what I ranted about in: > > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de > > > > I'm replacing the per-integer-size type checks with a single "int that > > fits in 32bit" check. This allows several lines of duplicated code to > > be removed. > > > > I haven't changed the macro names in the device code yet. If this > > series gets positive feedback then I'll send per-subsystem patches > > doing that. > > > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 > > Also tested: > > - migration-test --full --thorough > > - x86_64 compat run forwards and backwards for previous 3 QEMU releases > > - s390x compat run forwards and backwards for previous 2 QEMU releases > > - ppc64 compat run forwards and backwards for previous QEMU release > > - migration-test smoke ASAN/UBSAN run > > > > Fabiano Rosas (4): > > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE > > migration: Introduce VMStateOffset > > migration: Remove redundant flags > > migration: Remove duplicate vmstate macros > > Nice work! > > I think I was only looking at VBUFFER side and I thought it was fine > sticking with 32bit even signed or not, not a huge deal. But cleaning up > VARRAY whole thing together looks definitely an improvement. I definitely > like your version here. > > I assume with your series I can drop both of my patches here, right? > > [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc() > https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com > (I'll still respin with the rest) > > [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD > https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com > > The only missing piece would be an multiply overflow check in > vmstate_handle_alloc(), if you could add that check too while rewritting > that in patch 1 then I think it'll cover all. > > Vladimir's ask in the separate email makes sense: I wonder if we can also > do one step further and merge VBUFFER into VARRAY. > > The other trivial thing is, while looking, I found one trivial macro > VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether. Now officially declare support for u64 on all these offsets, we also need to double check on our alignment with security issues. Similar reports will not be a bug anymore but results will be the same I assume: it's anything the attacker can feed a u64 directly (instead of an int32_t negative overflow), result is still failing a malloc() with enormously large numbers, legally this time. Do you still plan to work on finding per-user upper limit or whatever of that kind? I'd say time spent working on series like this worths more than that, but I still want to check with you while looking at this solution. I suppose with this, one option is we can close all tickets reporting security issues while allocating with all u64 fields. Thanks, -- Peter Xu
Peter Xu <peterx@redhat.com> writes:
> On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
>> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
>> > Hi, this is basically what I ranted about in:
>> > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
>> >
>> > I'm replacing the per-integer-size type checks with a single "int that
>> > fits in 32bit" check. This allows several lines of duplicated code to
>> > be removed.
>> >
>> > I haven't changed the macro names in the device code yet. If this
>> > series gets positive feedback then I'll send per-subsystem patches
>> > doing that.
>> >
>> > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
>> > Also tested:
>> > - migration-test --full --thorough
>> > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
>> > - s390x compat run forwards and backwards for previous 2 QEMU releases
>> > - ppc64 compat run forwards and backwards for previous QEMU release
>> > - migration-test smoke ASAN/UBSAN run
>> >
>> > Fabiano Rosas (4):
>> > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
>> > migration: Introduce VMStateOffset
>> > migration: Remove redundant flags
>> > migration: Remove duplicate vmstate macros
>>
>> Nice work!
>>
>> I think I was only looking at VBUFFER side and I thought it was fine
>> sticking with 32bit even signed or not, not a huge deal. But cleaning up
>> VARRAY whole thing together looks definitely an improvement. I definitely
>> like your version here.
>>
>> I assume with your series I can drop both of my patches here, right?
>>
>> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
>> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
>> (I'll still respin with the rest)
>>
>> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
>> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
>>
>> The only missing piece would be an multiply overflow check in
>> vmstate_handle_alloc(), if you could add that check too while rewritting
>> that in patch 1 then I think it'll cover all.
>>
>> Vladimir's ask in the separate email makes sense: I wonder if we can also
>> do one step further and merge VBUFFER into VARRAY.
>>
>> The other trivial thing is, while looking, I found one trivial macro
>> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
>
> Now officially declare support for u64 on all these offsets, we also need
> to double check on our alignment with security issues.
>
> Similar reports will not be a bug anymore but results will be the same I
> assume: it's anything the attacker can feed a u64 directly (instead of an
> int32_t negative overflow), result is still failing a malloc() with
> enormously large numbers, legally this time.
>
Sprinkle some try_alloc maybe? Shouldn't be just a few from the
migration code itself.
> Do you still plan to work on finding per-user upper limit or whatever of
> that kind? I'd say time spent working on series like this worths more than
> that, but I still want to check with you while looking at this solution.
>
I thought of some things for the future, but I don't have anything
concrete, only experiments:
1) For all vmstate macros that take an unbounded size, length, etc, add
a new variant:
VMSTATE_FOO_BAR_V()
+VMSTATE_FOO_BAR_V_BOUNDED()
And allow the vmstate writer to provide a "Bounds" object (format to be
defined) and the vmstate core would just use that if it's there and
that's it. I still think that's a functionality the migration code
should have in its API.
2) Allowing a transfer limit in bytes to be set in QEMUFile, which can
be adjusted at will, maybe using some context-manager-like construct:
WITH_RX_CAP(f, 1024) {
qemu_get_byte();
qemu_get_byte();
vmstate_load_vmsd()
qemu_get_buf();
...
}
I'm still not sure how useful these would be.
> I suppose with this, one option is we can close all tickets reporting
> security issues while allocating with all u64 fields.
>
> Thanks,
On Thu, Jul 30, 2026 at 12:50:38PM -0300, Fabiano Rosas wrote:
> Peter Xu <peterx@redhat.com> writes:
>
> > On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote:
> >> On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote:
> >> > Hi, this is basically what I ranted about in:
> >> > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de
> >> >
> >> > I'm replacing the per-integer-size type checks with a single "int that
> >> > fits in 32bit" check. This allows several lines of duplicated code to
> >> > be removed.
> >> >
> >> > I haven't changed the macro names in the device code yet. If this
> >> > series gets positive feedback then I'll send per-subsystem patches
> >> > doing that.
> >> >
> >> > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081
> >> > Also tested:
> >> > - migration-test --full --thorough
> >> > - x86_64 compat run forwards and backwards for previous 3 QEMU releases
> >> > - s390x compat run forwards and backwards for previous 2 QEMU releases
> >> > - ppc64 compat run forwards and backwards for previous QEMU release
> >> > - migration-test smoke ASAN/UBSAN run
> >> >
> >> > Fabiano Rosas (4):
> >> > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE
> >> > migration: Introduce VMStateOffset
> >> > migration: Remove redundant flags
> >> > migration: Remove duplicate vmstate macros
> >>
> >> Nice work!
> >>
> >> I think I was only looking at VBUFFER side and I thought it was fine
> >> sticking with 32bit even signed or not, not a huge deal. But cleaning up
> >> VARRAY whole thing together looks definitely an improvement. I definitely
> >> like your version here.
> >>
> >> I assume with your series I can drop both of my patches here, right?
> >>
> >> [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc()
> >> https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com
> >> (I'll still respin with the rest)
> >>
> >> [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD
> >> https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com
> >>
> >> The only missing piece would be an multiply overflow check in
> >> vmstate_handle_alloc(), if you could add that check too while rewritting
> >> that in patch 1 then I think it'll cover all.
> >>
> >> Vladimir's ask in the separate email makes sense: I wonder if we can also
> >> do one step further and merge VBUFFER into VARRAY.
> >>
> >> The other trivial thing is, while looking, I found one trivial macro
> >> VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether.
> >
> > Now officially declare support for u64 on all these offsets, we also need
> > to double check on our alignment with security issues.
> >
> > Similar reports will not be a bug anymore but results will be the same I
> > assume: it's anything the attacker can feed a u64 directly (instead of an
> > int32_t negative overflow), result is still failing a malloc() with
> > enormously large numbers, legally this time.
> >
>
> Sprinkle some try_alloc maybe? Shouldn't be just a few from the
> migration code itself.
It's a matter of how helpful is it to fail slightly more gracefully,
v.s. malloc()'s error message.
>
> > Do you still plan to work on finding per-user upper limit or whatever of
> > that kind? I'd say time spent working on series like this worths more than
> > that, but I still want to check with you while looking at this solution.
> >
>
> I thought of some things for the future, but I don't have anything
> concrete, only experiments:
>
> 1) For all vmstate macros that take an unbounded size, length, etc, add
> a new variant:
>
> VMSTATE_FOO_BAR_V()
> +VMSTATE_FOO_BAR_V_BOUNDED()
>
> And allow the vmstate writer to provide a "Bounds" object (format to be
> defined) and the vmstate core would just use that if it's there and
> that's it. I still think that's a functionality the migration code
> should have in its API.
Something relevant I just sent:
https://lore.kernel.org/r/amt4Yv8miJ_rFYSS@x1.local
Based on that, IMHO what we need in a more useful way is, again taking
gtree example, a wrapper to g_tree_new_full() but allow specifying a size..
then gracefully allow failure to happen.
Maybe it's not easy to wrap it to make it graceful and generic, the whole
point is for the existing unlimited resources problem with migration we
should limit it from the user before migration even starts.
>
> 2) Allowing a transfer limit in bytes to be set in QEMUFile, which can
> be adjusted at will, maybe using some context-manager-like construct:
>
> WITH_RX_CAP(f, 1024) {
> qemu_get_byte();
> qemu_get_byte();
> vmstate_load_vmsd()
> qemu_get_buf();
> ...
> }
>
> I'm still not sure how useful these would be.
>
> > I suppose with this, one option is we can close all tickets reporting
> > security issues while allocating with all u64 fields.
> >
> > Thanks,
>
--
Peter Xu
On Thu, Jul 30, 2026 at 10:22:28AM -0400, Peter Xu wrote: > On Thu, Jul 30, 2026 at 10:09:30AM -0400, Peter Xu wrote: > > On Wed, Jul 29, 2026 at 07:52:23PM -0300, Fabiano Rosas wrote: > > > Hi, this is basically what I ranted about in: > > > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de > > > > > > I'm replacing the per-integer-size type checks with a single "int that > > > fits in 32bit" check. This allows several lines of duplicated code to > > > be removed. > > > > > > I haven't changed the macro names in the device code yet. If this > > > series gets positive feedback then I'll send per-subsystem patches > > > doing that. > > > > > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 > > > Also tested: > > > - migration-test --full --thorough > > > - x86_64 compat run forwards and backwards for previous 3 QEMU releases > > > - s390x compat run forwards and backwards for previous 2 QEMU releases > > > - ppc64 compat run forwards and backwards for previous QEMU release > > > - migration-test smoke ASAN/UBSAN run > > > > > > Fabiano Rosas (4): > > > migration: Remove VMSTATE_ARRAY_INT32_UNSAFE > > > migration: Introduce VMStateOffset > > > migration: Remove redundant flags > > > migration: Remove duplicate vmstate macros > > > > Nice work! > > > > I think I was only looking at VBUFFER side and I thought it was fine > > sticking with 32bit even signed or not, not a huge deal. But cleaning up > > VARRAY whole thing together looks definitely an improvement. I definitely > > like your version here. > > > > I assume with your series I can drop both of my patches here, right? > > > > [PATCH v2 1/5] migration: Fix possible overflow in vmstate_handle_alloc() > > https://lore.kernel.org/r/20260728210417.1925078-2-peterx@redhat.com > > (I'll still respin with the rest) > > > > [PATCH] vhost/migration: Fix incorrect size used in inflight->addr in VMSD > > https://lore.kernel.org/r/20260728153942.1891677-1-peterx@redhat.com > > > > The only missing piece would be an multiply overflow check in > > vmstate_handle_alloc(), if you could add that check too while rewritting > > that in patch 1 then I think it'll cover all. > > > > Vladimir's ask in the separate email makes sense: I wonder if we can also > > do one step further and merge VBUFFER into VARRAY. > > > > The other trivial thing is, while looking, I found one trivial macro > > VMSTATE_PARTIAL_VBUFFER not used; can drop it altogether. > > Now officially declare support for u64 on all these offsets, we also need > to double check on our alignment with security issues. > Similar reports will not be a bug anymore but results will be the same I > assume: it's anything the attacker can feed a u64 directly (instead of an > int32_t negative overflow), result is still failing a malloc() with > enormously large numbers, legally this time. > > Do you still plan to work on finding per-user upper limit or whatever of > that kind? I'd say time spent working on series like this worths more than > that, but I still want to check with you while looking at this solution. > > I suppose with this, one option is we can close all tickets reporting > security issues while allocating with all u64 fields. All issues where migration stream is malformed aren't security issues according to the current policy. Whether to close these or use them as a starting point in research, is up to you guys. > > Thanks, > > -- > Peter Xu
On 30.07.26 01:52, Fabiano Rosas wrote: > Hi, this is basically what I ranted about in: > https://lore.kernel.org/r/87jyqeomqz.fsf@suse.de > > I'm replacing the per-integer-size type checks with a single "int that > fits in 32bit" check. This allows several lines of duplicated code to > be removed. > > I haven't changed the macro names in the device code yet. If this > series gets positive feedback then I'll send per-subsystem patches > doing that. > > CI run: https://gitlab.com/farosas/qemu/-/pipelines/2716814081 > Also tested: > - migration-test --full --thorough > - x86_64 compat run forwards and backwards for previous 3 QEMU releases > - s390x compat run forwards and backwards for previous 2 QEMU releases > - ppc64 compat run forwards and backwards for previous QEMU release > - migration-test smoke ASAN/UBSAN run > Thanks for doing it, all these _TYPE_ were pain! -- Best regards, Vladimir
© 2016 - 2026 Red Hat, Inc.