Re: [PATCH] virtio: stop migrating num_default, validate vring.num on load

On Tue, Jul 28, 2026 at 11:55:41AM +0300, Michael Tokarev wrote:
> On 7/24/26 14:58, Michael S. Tsirkin wrote:
> > num_default tracks the allocation size of used_elems, set by
> > virtio_add_queue(). Migrating it via the ringsize subsection is
> > wrong: a migration stream (malicious or simply from a different
> > configuration) can inflate num_default so that
> > virtio_queue_set_num() accepts oversized values, leading to OOB
> > access on the used_elems array.
> > 
> > It is not even migrated consistently: a configuration with a
> > smaller num_default could thinkably migrate and work but in the
> > common case of num == num_default the value is not actually sent.
> > 
> > Stop migrating num_default: make virtio_ringsize_needed() return
> > false so the subsection is never sent, and use VMSTATE_UNUSED to
> > consume the field from old streams without applying it. The
> > destination keeps its local num_default from virtio_add_queue(),
> > which matches the actual allocation.
> 
> What happens here with an attempt to migrate to an earlier
> version of qemu, without this patch applied, but with non-
> default values of vring.num?


It just works.

> Maybe we should send the values still, but always validate
> them on load?
>
> Also, what should we do here on older (stable) releases?
> It raises the same question, how about migrating back to
> a version without this change?
> 
> Thanks,
> 
> /mjt

We should never have sent them. Skipping the value works fine.

> > Also validate vring.num against num_default when loading the core
> > virtio state, rejecting streams that supply a queue size larger
> > than the locally allocated maximum.
> > 
> > Fixes: 46c5d0823d ("virtio: ring sizes vs. reset")
> > Fixes: 50e5ae4dc3 ("migration/virtio: Remove simple .get/.put use")
> > Cc: Cornelia Huck <[email protected]>
> > Cc: Peter Maydell <[email protected]>
> > Signed-off-by: Michael S. Tsirkin <[email protected]>
> > ---
> >   hw/virtio/virtio.c | 16 +++++++---------
> >   1 file changed, 7 insertions(+), 9 deletions(-)
> > 
> > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > index 94ddbfd09f..437c001a04 100644
> > --- a/hw/virtio/virtio.c
> > +++ b/hw/virtio/virtio.c
> > @@ -2817,14 +2817,6 @@ static bool virtio_packed_virtqueue_needed(void *opaque)
> >   static bool virtio_ringsize_needed(void *opaque)
> >   {
> > -    VirtIODevice *vdev = opaque;
> > -    int i;
> > -
> > -    for (i = 0; i < VIRTIO_QUEUE_MAX; i++) {
> > -        if (vdev->vq[i].vring.num != vdev->vq[i].vring.num_default) {
> > -            return true;
> > -        }
> > -    }
> >       return false;
> >   }
> > @@ -2913,7 +2905,7 @@ static const VMStateDescription vmstate_ringsize = {
> >       .version_id = 1,
> >       .minimum_version_id = 1,
> >       .fields = (const VMStateField[]) {
> > -        VMSTATE_UINT32(vring.num_default, struct VirtQueue),
> > +        VMSTATE_UNUSED(sizeof(uint32_t)),
> >           VMSTATE_END_OF_LIST()
> >       }
> >   };
> > @@ -3593,6 +3585,12 @@ virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
> >       for (i = 0; i < num; i++) {
> >           vdev->vq[i].vring.num = qemu_get_be32(f);
> > +        if (vdev->vq[i].vring.num > vdev->vq[i].vring.num_default) {
> > +            error_report("VQ %d vring.num %u exceeds allocated max %u",
> > +                         i, vdev->vq[i].vring.num,
> > +                         vdev->vq[i].vring.num_default);
> > +            return -1;
> > +        }
> >           if (k->has_variable_vring_alignment) {
> >               vdev->vq[i].vring.align = qemu_get_be32(f);
> >           }