[v2] hw/display/virtio-gpu: Reject non-migratable large blobs

[PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs

Posted by Akihiko Odaki 2 months ago

The blob size is encoded as a 64-bit integer in the virtio protocol, but
it is encoded as a 32-bit integer in the migration stream, and
a large blob whose size cannot be expressed in a 32-bit integer will
be corrupted after migration. Deny creating such large blobs.

Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection")
Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"")
Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
Changes in v2:
- Add the Fixes: tag for f66767f75c9c.
- Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob
  VMState version 2.
- Link to v1: https://lore.kernel.org/qemu-devel/20260725-blob-v1-1-f643fbd76398@rsg.ci.i.u-tokyo.ac.jp
---
 hw/display/virtio-gpu.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 718ba3039290..987b1b3ea860 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
         return;
     }
 
+    if (cblob.size > UINT32_MAX) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n",
+                      __func__, cblob.size, UINT32_MAX);
+        cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
+        return;
+    }
+
     if (virtio_gpu_find_resource(g, cblob.resource_id)) {
         qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
                       __func__, cblob.resource_id);
@@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
         }
         assert(!res->image);
         qemu_put_be32(f, res->resource_id);
+
+        /*
+         * The migration stream encodes blob_size as a 32-bit integer for
+         * compatibility though virtio encodes it as a 64-bit integer. A newer
+         * version of the migration stream should encode it as a 64-bit integer.
+         */
         qemu_put_be32(f, res->blob_size);
+
         qemu_put_be32(f, res->iov_cnt);
         for (i = 0; i < res->iov_cnt; i++) {
             qemu_put_be64(f, res->addrs[i]);

---
base-commit: 006a22cb26998998385b104db1ff9466ef2f3153
change-id: 20260725-blob-8a6a1a85601a

Best regards,
--  
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

Re: [PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs

Posted by Daniel P. Berrangé 2 months ago

On Mon, Jul 27, 2026 at 06:23:25PM +0900, Akihiko Odaki wrote:
> The blob size is encoded as a 64-bit integer in the virtio protocol, but
> it is encoded as a 32-bit integer in the migration stream, and
> a large blob whose size cannot be expressed in a 32-bit integer will
> be corrupted after migration. Deny creating such large blobs.

Is there a valid use case for such large blobs ?   If so, rather than
rejecting the blob, we could dynamically add a migration  blocker.
That would allow the valid use cases, while preventing the migration
problem.

> 
> Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection")
> Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"")
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> ---
> Changes in v2:
> - Add the Fixes: tag for f66767f75c9c.
> - Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob
>   VMState version 2.
> - Link to v1: https://lore.kernel.org/qemu-devel/20260725-blob-v1-1-f643fbd76398@rsg.ci.i.u-tokyo.ac.jp
> ---
>  hw/display/virtio-gpu.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 718ba3039290..987b1b3ea860 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
>          return;
>      }
>  
> +    if (cblob.size > UINT32_MAX) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n",
> +                      __func__, cblob.size, UINT32_MAX);
> +        cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
> +        return;
> +    }
> +
>      if (virtio_gpu_find_resource(g, cblob.resource_id)) {
>          qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
>                        __func__, cblob.resource_id);
> @@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
>          }
>          assert(!res->image);
>          qemu_put_be32(f, res->resource_id);
> +
> +        /*
> +         * The migration stream encodes blob_size as a 32-bit integer for
> +         * compatibility though virtio encodes it as a 64-bit integer. A newer
> +         * version of the migration stream should encode it as a 64-bit integer.
> +         */
>          qemu_put_be32(f, res->blob_size);
> +
>          qemu_put_be32(f, res->iov_cnt);
>          for (i = 0; i < res->iov_cnt; i++) {
>              qemu_put_be64(f, res->addrs[i]);
> 
> ---
> base-commit: 006a22cb26998998385b104db1ff9466ef2f3153
> change-id: 20260725-blob-8a6a1a85601a
> 
> Best regards,
> --  
> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|

Re: [PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs

Posted by Akihiko Odaki 2 months ago

On 2026/07/27 18:30, Daniel P. Berrangé wrote:
> On Mon, Jul 27, 2026 at 06:23:25PM +0900, Akihiko Odaki wrote:
>> The blob size is encoded as a 64-bit integer in the virtio protocol, but
>> it is encoded as a 32-bit integer in the migration stream, and
>> a large blob whose size cannot be expressed in a 32-bit integer will
>> be corrupted after migration. Deny creating such large blobs.
> 
> Is there a valid use case for such large blobs ?   If so, rather than
> rejecting the blob, we could dynamically add a migration  blocker.
> That would allow the valid use cases, while preventing the migration
> problem.

For the QEMU code path changed here, there should be no practical use 
case for a blob larger than 4 GiB. The non-virgl implementation uses 
blobs as scanout or cursor backing. Such a size is excessive for those uses.

A dynamic migration blocker would also let the guest make an otherwise 
migratable VM non-migratable at runtime merely by creating such a 
resource. This could interfere with host-level VM management. So I think 
rejecting sizes that the current migration format cannot represent is 
better for now.

If a concrete use case emerges, extending the migration format would be 
preferable to making migratability depend on guest-controlled state.

Regards,
Akihiko Odaki

> 
>>
>> Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection")
>> Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"")
>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>> ---
>> Changes in v2:
>> - Add the Fixes: tag for f66767f75c9c.
>> - Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob
>>    VMState version 2.
>> - Link to v1: https://lore.kernel.org/qemu-devel/20260725-blob-v1-1-f643fbd76398@rsg.ci.i.u-tokyo.ac.jp
>> ---
>>   hw/display/virtio-gpu.c | 15 +++++++++++++++
>>   1 file changed, 15 insertions(+)
>>
>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>> index 718ba3039290..987b1b3ea860 100644
>> --- a/hw/display/virtio-gpu.c
>> +++ b/hw/display/virtio-gpu.c
>> @@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
>>           return;
>>       }
>>   
>> +    if (cblob.size > UINT32_MAX) {
>> +        qemu_log_mask(LOG_GUEST_ERROR,
>> +                      "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n",
>> +                      __func__, cblob.size, UINT32_MAX);
>> +        cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
>> +        return;
>> +    }
>> +
>>       if (virtio_gpu_find_resource(g, cblob.resource_id)) {
>>           qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
>>                         __func__, cblob.resource_id);
>> @@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
>>           }
>>           assert(!res->image);
>>           qemu_put_be32(f, res->resource_id);
>> +
>> +        /*
>> +         * The migration stream encodes blob_size as a 32-bit integer for
>> +         * compatibility though virtio encodes it as a 64-bit integer. A newer
>> +         * version of the migration stream should encode it as a 64-bit integer.
>> +         */
>>           qemu_put_be32(f, res->blob_size);
>> +
>>           qemu_put_be32(f, res->iov_cnt);
>>           for (i = 0; i < res->iov_cnt; i++) {
>>               qemu_put_be64(f, res->addrs[i]);
>>
>> ---
>> base-commit: 006a22cb26998998385b104db1ff9466ef2f3153
>> change-id: 20260725-blob-8a6a1a85601a
>>
>> Best regards,
>> --
>> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>
>>
> 
> With regards,
> Daniel

Re: [PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs

Posted by Marc-André Lureau 2 months ago

Hi

On Tue, Jul 28, 2026 at 10:26 AM Akihiko Odaki
<odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>
> On 2026/07/27 18:30, Daniel P. Berrangé wrote:
> > On Mon, Jul 27, 2026 at 06:23:25PM +0900, Akihiko Odaki wrote:
> >> The blob size is encoded as a 64-bit integer in the virtio protocol, but
> >> it is encoded as a 32-bit integer in the migration stream, and
> >> a large blob whose size cannot be expressed in a 32-bit integer will
> >> be corrupted after migration. Deny creating such large blobs.
> >
> > Is there a valid use case for such large blobs ?   If so, rather than
> > rejecting the blob, we could dynamically add a migration  blocker.
> > That would allow the valid use cases, while preventing the migration
> > problem.
>
> For the QEMU code path changed here, there should be no practical use
> case for a blob larger than 4 GiB. The non-virgl implementation uses
> blobs as scanout or cursor backing. Such a size is excessive for those uses.
>
> A dynamic migration blocker would also let the guest make an otherwise
> migratable VM non-migratable at runtime merely by creating such a
> resource. This could interfere with host-level VM management. So I think
> rejecting sizes that the current migration format cannot represent is
> better for now.

What's the difference? The dynamic blocker would simply happen earlier
or be queryable, no?

> If a concrete use case emerges, extending the migration format would be
> preferable to making migratability depend on guest-controlled state.
>
> Regards,
> Akihiko Odaki
>
> >
> >>
> >> Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection")
> >> Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"")
> >> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >> ---
> >> Changes in v2:
> >> - Add the Fixes: tag for f66767f75c9c.
> >> - Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob
> >>    VMState version 2.
> >> - Link to v1: https://lore.kernel.org/qemu-devel/20260725-blob-v1-1-f643fbd76398@rsg.ci.i.u-tokyo.ac.jp
> >> ---
> >>   hw/display/virtio-gpu.c | 15 +++++++++++++++
> >>   1 file changed, 15 insertions(+)
> >>
> >> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> >> index 718ba3039290..987b1b3ea860 100644
> >> --- a/hw/display/virtio-gpu.c
> >> +++ b/hw/display/virtio-gpu.c
> >> @@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
> >>           return;
> >>       }
> >>
> >> +    if (cblob.size > UINT32_MAX) {
> >> +        qemu_log_mask(LOG_GUEST_ERROR,
> >> +                      "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n",
> >> +                      __func__, cblob.size, UINT32_MAX);
> >> +        cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
> >> +        return;
> >> +    }
> >> +
> >>       if (virtio_gpu_find_resource(g, cblob.resource_id)) {
> >>           qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
> >>                         __func__, cblob.resource_id);
> >> @@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
> >>           }
> >>           assert(!res->image);
> >>           qemu_put_be32(f, res->resource_id);
> >> +
> >> +        /*
> >> +         * The migration stream encodes blob_size as a 32-bit integer for
> >> +         * compatibility though virtio encodes it as a 64-bit integer. A newer
> >> +         * version of the migration stream should encode it as a 64-bit integer.
> >> +         */
> >>           qemu_put_be32(f, res->blob_size);
> >> +
> >>           qemu_put_be32(f, res->iov_cnt);
> >>           for (i = 0; i < res->iov_cnt; i++) {
> >>               qemu_put_be64(f, res->addrs[i]);
> >>
> >> ---
> >> base-commit: 006a22cb26998998385b104db1ff9466ef2f3153
> >> change-id: 20260725-blob-8a6a1a85601a
> >>
> >> Best regards,
> >> --
> >> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
> >>
> >>
> >
> > With regards,
> > Daniel
>
>


-- 
Marc-André Lureau

Re: [PATCH v2] hw/display/virtio-gpu: Reject non-migratable large blobs

Posted by Akihiko Odaki 2 months ago

On 2026/07/28 16:04, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jul 28, 2026 at 10:26 AM Akihiko Odaki
> <odaki@rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> On 2026/07/27 18:30, Daniel P. Berrangé wrote:
>>> On Mon, Jul 27, 2026 at 06:23:25PM +0900, Akihiko Odaki wrote:
>>>> The blob size is encoded as a 64-bit integer in the virtio protocol, but
>>>> it is encoded as a 32-bit integer in the migration stream, and
>>>> a large blob whose size cannot be expressed in a 32-bit integer will
>>>> be corrupted after migration. Deny creating such large blobs.
>>>
>>> Is there a valid use case for such large blobs ?   If so, rather than
>>> rejecting the blob, we could dynamically add a migration  blocker.
>>> That would allow the valid use cases, while preventing the migration
>>> problem.
>>
>> For the QEMU code path changed here, there should be no practical use
>> case for a blob larger than 4 GiB. The non-virgl implementation uses
>> blobs as scanout or cursor backing. Such a size is excessive for those uses.
>>
>> A dynamic migration blocker would also let the guest make an otherwise
>> migratable VM non-migratable at runtime merely by creating such a
>> resource. This could interfere with host-level VM management. So I think
>> rejecting sizes that the current migration format cannot represent is
>> better for now.
> 
> What's the difference? The dynamic blocker would simply happen earlier
> or be queryable, no?

The difference I had in mind was that the guest could make migration 
unavailable after the VM had started in an unexpected way for the user 
who manages the host.

But I now remember that, if QEMU is started with -only-migratable, 
migrate_add_blocker() fails, so resource creation can be rejected in 
that configuration. It also fails while a migration or snapshot is 
already in progress.

Now I feel safer with the dynamic blocker. I will change the patch to 
keep a migration blocker installed while any such blob exists.

Regards,
Akihiko Odaki

> 
>> If a concrete use case emerges, extending the migration format would be
>> preferable to making migratability depend on guest-controlled state.
>>
>> Regards,
>> Akihiko Odaki
>>
>>>
>>>>
>>>> Fixes: f66767f75c9c ("virtio-gpu: add virtio-gpu/blob vmstate subsection")
>>>> Fixes: 10b9ddbc83b9 ("Revert "virtio-gpu: block migration of VMs with blob=true"")
>>>> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>> ---
>>>> Changes in v2:
>>>> - Add the Fixes: tag for f66767f75c9c.
>>>> - Add a TODO to encode blob_size as 64 bits in virtio-gpu/blob
>>>>     VMState version 2.
>>>> - Link to v1: https://lore.kernel.org/qemu-devel/20260725-blob-v1-1-f643fbd76398@rsg.ci.i.u-tokyo.ac.jp
>>>> ---
>>>>    hw/display/virtio-gpu.c | 15 +++++++++++++++
>>>>    1 file changed, 15 insertions(+)
>>>>
>>>> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
>>>> index 718ba3039290..987b1b3ea860 100644
>>>> --- a/hw/display/virtio-gpu.c
>>>> +++ b/hw/display/virtio-gpu.c
>>>> @@ -352,6 +352,14 @@ static void virtio_gpu_resource_create_blob(VirtIOGPU *g,
>>>>            return;
>>>>        }
>>>>
>>>> +    if (cblob.size > UINT32_MAX) {
>>>> +        qemu_log_mask(LOG_GUEST_ERROR,
>>>> +                      "%s: blob too large (%" PRIu64 "> %" PRIu32 ")\n",
>>>> +                      __func__, cblob.size, UINT32_MAX);
>>>> +        cmd->error = VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY;
>>>> +        return;
>>>> +    }
>>>> +
>>>>        if (virtio_gpu_find_resource(g, cblob.resource_id)) {
>>>>            qemu_log_mask(LOG_GUEST_ERROR, "%s: resource already exists %d\n",
>>>>                          __func__, cblob.resource_id);
>>>> @@ -1435,7 +1443,14 @@ static int virtio_gpu_blob_save(QEMUFile *f, void *opaque, size_t size,
>>>>            }
>>>>            assert(!res->image);
>>>>            qemu_put_be32(f, res->resource_id);
>>>> +
>>>> +        /*
>>>> +         * The migration stream encodes blob_size as a 32-bit integer for
>>>> +         * compatibility though virtio encodes it as a 64-bit integer. A newer
>>>> +         * version of the migration stream should encode it as a 64-bit integer.
>>>> +         */
>>>>            qemu_put_be32(f, res->blob_size);
>>>> +
>>>>            qemu_put_be32(f, res->iov_cnt);
>>>>            for (i = 0; i < res->iov_cnt; i++) {
>>>>                qemu_put_be64(f, res->addrs[i]);
>>>>
>>>> ---
>>>> base-commit: 006a22cb26998998385b104db1ff9466ef2f3153
>>>> change-id: 20260725-blob-8a6a1a85601a
>>>>
>>>> Best regards,
>>>> --
>>>> Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
>>>>
>>>>
>>>
>>> With regards,
>>> Daniel
>>
>>
> 
>