hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

[PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Laurent Vivier 2 weeks, 3 days ago

The migration restore path reads nr_active_ports from the incoming
stream and passes it directly to fetch_active_ports_list(), which
uses it to size a heap allocation. A crafted migration stream can set
this field to a very large value, causing QEMU to attempt a
multi-gigabyte allocation and abort.

Fix this by checking nr_active_ports against the configured
max_virtserial_ports before calling fetch_active_ports_list().

Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
---
 hw/char/virtio-serial-bus.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index c1973f0248fc..80f1b308aa53 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
 
     qemu_get_be32s(f, &nr_active_ports);
 
+    if (nr_active_ports > max_nr_ports) {
+        return -EINVAL;
+    }
+
     if (nr_active_ports) {
         ret = fetch_active_ports_list(f, s, nr_active_ports);
         if (ret) {
-- 
2.54.0

Re: [PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Michael S. Tsirkin 2 weeks, 3 days ago

On Wed, Jul 08, 2026 at 04:41:12PM +0200, Laurent Vivier wrote:
> The migration restore path reads nr_active_ports from the incoming
> stream and passes it directly to fetch_active_ports_list(), which
> uses it to size a heap allocation. A crafted migration stream can set
> this field to a very large value, causing QEMU to attempt a
> multi-gigabyte allocation and abort.
> 
> Fix this by checking nr_active_ports against the configured
> max_virtserial_ports before calling fetch_active_ports_list().
> 
> Cc: qemu-stable@nongnu.org

Can you add a Fixes tag pls?

> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

to make sure who is merging this me?

> ---
>  hw/char/virtio-serial-bus.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index c1973f0248fc..80f1b308aa53 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
>  
>      qemu_get_be32s(f, &nr_active_ports);
>  
> +    if (nr_active_ports > max_nr_ports) {
> +        return -EINVAL;
> +    }
> +
>      if (nr_active_ports) {
>          ret = fetch_active_ports_list(f, s, nr_active_ports);
>          if (ret) {
> -- 
> 2.54.0

Re: [PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Laurent Vivier 2 weeks, 3 days ago

On 7/9/26 07:55, Michael S. Tsirkin wrote:
> On Wed, Jul 08, 2026 at 04:41:12PM +0200, Laurent Vivier wrote:
>> The migration restore path reads nr_active_ports from the incoming
>> stream and passes it directly to fetch_active_ports_list(), which
>> uses it to size a heap allocation. A crafted migration stream can set
>> this field to a very large value, causing QEMU to attempt a
>> multi-gigabyte allocation and abort.
>>
>> Fix this by checking nr_active_ports against the configured
>> max_virtserial_ports before calling fetch_active_ports_list().
>>
>> Cc: qemu-stable@nongnu.org
> 
> Can you add a Fixes tag pls?

Fixes: 6663a1956eb6 ("virtio-serial-bus: Maintain guest and host port open/close state")

> 
>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> 
> to make sure who is merging this me?

Yes, please merge it through your tree

Thanks,
Laurent

> 
>> ---
>>   hw/char/virtio-serial-bus.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
>> index c1973f0248fc..80f1b308aa53 100644
>> --- a/hw/char/virtio-serial-bus.c
>> +++ b/hw/char/virtio-serial-bus.c
>> @@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
>>   
>>       qemu_get_be32s(f, &nr_active_ports);
>>   
>> +    if (nr_active_ports > max_nr_ports) {
>> +        return -EINVAL;
>> +    }
>> +
>>       if (nr_active_ports) {
>>           ret = fetch_active_ports_list(f, s, nr_active_ports);
>>           if (ret) {
>> -- 
>> 2.54.0
>

Re: [PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Michael S. Tsirkin 2 weeks, 3 days ago

On Wed, Jul 08, 2026 at 04:41:12PM +0200, Laurent Vivier wrote:
> The migration restore path reads nr_active_ports from the incoming
> stream and passes it directly to fetch_active_ports_list(), which
> uses it to size a heap allocation. A crafted migration stream can set
> this field to a very large value, causing QEMU to attempt a
> multi-gigabyte allocation and abort.
> 
> Fix this by checking nr_active_ports against the configured
> max_virtserial_ports before calling fetch_active_ports_list().
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>

Acked-by: Michael S. Tsirkin <mst@redhat.com>


> ---
>  hw/char/virtio-serial-bus.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index c1973f0248fc..80f1b308aa53 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
>  
>      qemu_get_be32s(f, &nr_active_ports);
>  
> +    if (nr_active_ports > max_nr_ports) {
> +        return -EINVAL;
> +    }
> +
>      if (nr_active_ports) {
>          ret = fetch_active_ports_list(f, s, nr_active_ports);
>          if (ret) {
> -- 
> 2.54.0

Re: [PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Thomas Huth 2 weeks, 3 days ago

On 08/07/2026 16.41, Laurent Vivier wrote:
> The migration restore path reads nr_active_ports from the incoming
> stream and passes it directly to fetch_active_ports_list(), which
> uses it to size a heap allocation. A crafted migration stream can set
> this field to a very large value, causing QEMU to attempt a
> multi-gigabyte allocation and abort.
> 
> Fix this by checking nr_active_ports against the configured
> max_virtserial_ports before calling fetch_active_ports_list().
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>   hw/char/virtio-serial-bus.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index c1973f0248fc..80f1b308aa53 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -804,6 +804,10 @@ static int virtio_serial_load_device(VirtIODevice *vdev, QEMUFile *f,
>   
>       qemu_get_be32s(f, &nr_active_ports);
>   
> +    if (nr_active_ports > max_nr_ports) {
> +        return -EINVAL;
> +    }
> +
>       if (nr_active_ports) {
>           ret = fetch_active_ports_list(f, s, nr_active_ports);
>           if (ret) {

Reviewed-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH] hw/char/virtio-serial-bus: validate nr_active_ports from migration stream

Posted by Daniel P. Berrangé 2 weeks, 3 days ago

On Wed, Jul 08, 2026 at 04:41:12PM +0200, Laurent Vivier wrote:
> The migration restore path reads nr_active_ports from the incoming
> stream and passes it directly to fetch_active_ports_list(), which
> uses it to size a heap allocation. A crafted migration stream can set
> this field to a very large value, causing QEMU to attempt a
> multi-gigabyte allocation and abort.
> 
> Fix this by checking nr_active_ports against the configured
> max_virtserial_ports before calling fetch_active_ports_list().
> 
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3801
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/char/virtio-serial-bus.c | 4 ++++
>  1 file changed, 4 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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 :|