From: Peter Xu <peterx@redhat.com>
To: Fabiano Rosas <farosas@suse.de>
Cc: qemu-devel@nongnu.org, "Michael S . Tsirkin" <mst@redhat.com>,
Alexandr Moshkov <dtalexundeer@yandex-team.ru>,
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Subject: Re: [PATCH 0/4] migration: Remove extra type-checking from vmstate macros
Date: Thu, 30 Jul 2026 10:22:28 -0400 [thread overview]
Message-ID: <amteJEbrPguzFpH5@x1.local> (raw)
In-Reply-To: <amtbGkfKvPCJgDec@x1.local>
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
next prev parent reply other threads:[~2026-07-30 14:23 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-29 22:52 [PATCH 0/4] migration: Remove extra type-checking from vmstate macros Fabiano Rosas
2026-07-29 22:52 ` [PATCH 1/4] migration: Remove VMSTATE_ARRAY_INT32_UNSAFE Fabiano Rosas
2026-07-30 8:07 ` Vladimir Sementsov-Ogievskiy
2026-07-29 22:52 ` [PATCH 2/4] migration: Introduce VMStateOffset Fabiano Rosas
2026-07-30 8:06 ` Vladimir Sementsov-Ogievskiy
2026-07-30 14:45 ` Fabiano Rosas
2026-07-30 14:35 ` Michael S. Tsirkin
2026-07-30 15:15 ` Fabiano Rosas
2026-07-30 15:23 ` Peter Xu
2026-07-29 22:52 ` [PATCH 3/4] migration: Remove redundant flags Fabiano Rosas
2026-07-30 8:11 ` Vladimir Sementsov-Ogievskiy
2026-07-30 8:23 ` Vladimir Sementsov-Ogievskiy
2026-07-29 22:52 ` [PATCH 4/4] migration: Remove duplicate vmstate macros Fabiano Rosas
2026-07-30 8:19 ` Vladimir Sementsov-Ogievskiy
2026-07-30 8:22 ` [PATCH 0/4] migration: Remove extra type-checking from " Vladimir Sementsov-Ogievskiy
2026-07-30 14:09 ` Peter Xu
2026-07-30 14:22 ` Peter Xu [this message]
2026-07-30 14:45 ` Michael S. Tsirkin
2026-07-30 15:50 ` Fabiano Rosas
2026-07-30 16:31 ` Peter Xu
2026-07-30 15:37 ` Fabiano Rosas
2026-07-30 16:16 ` Peter Xu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=amteJEbrPguzFpH5@x1.local \
--to=peterx@redhat.com \
--cc=dtalexundeer@yandex-team.ru \
--cc=farosas@suse.de \
--cc=mst@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=vsementsov@yandex-team.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.