[PATCH] hw/char/virtio-serial-bus: fix guest-triggerable OOM in control_out()

[PATCH] hw/char/virtio-serial-bus: fix guest-triggerable OOM in control_out()

[Laurent Vivier]

A malicious guest can craft virtqueue descriptors with arbitrary lengths.
control_out() calls iov_size() on the guest-supplied scatter-gather list
and passes the result directly to g_malloc(), allowing a guest to force
QEMU to attempt multi-gigabyte allocations and crash the host process.

Fix this by copying at most sizeof(struct virtio_console_control) into a
stack-local variable instead of allocating a buffer sized by the guest.
handle_control_message() only accesses the fixed-size id, event, and
value fields, so no data beyond the struct was ever needed.

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

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index cd234dc6db1d..c1973f0248fc 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -344,22 +344,16 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
 }
 
 /* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
+static void handle_control_message(VirtIOSerial *vser,
+                                   struct virtio_console_control *gcpkt)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(vser);
     struct VirtIOSerialPort *port;
     VirtIOSerialPortClass *vsc;
-    struct virtio_console_control cpkt, *gcpkt;
+    struct virtio_console_control cpkt;
     uint8_t *buffer;
     size_t buffer_len;
 
-    gcpkt = buf;
-
-    if (len < sizeof(cpkt)) {
-        /* The guest sent an invalid control packet */
-        return;
-    }
-
     cpkt.event = virtio_lduw_p(vdev, &gcpkt->event);
     cpkt.value = virtio_lduw_p(vdev, &gcpkt->value);
 
@@ -457,41 +451,27 @@ static void control_in(VirtIODevice *vdev, VirtQueue *vq)
 
 static void control_out(VirtIODevice *vdev, VirtQueue *vq)
 {
+    struct virtio_console_control cpkt;
     VirtQueueElement *elem;
     VirtIOSerial *vser;
-    uint8_t *buf;
     size_t len;
 
     vser = VIRTIO_SERIAL(vdev);
 
-    len = 0;
-    buf = NULL;
     for (;;) {
-        size_t cur_len;
-
         elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
         if (!elem) {
             break;
         }
 
-        cur_len = iov_size(elem->out_sg, elem->out_num);
-        /*
-         * Allocate a new buf only if we didn't have one previously or
-         * if the size of the buf differs
-         */
-        if (cur_len > len) {
-            g_free(buf);
-
-            buf = g_malloc(cur_len);
-            len = cur_len;
+        len = iov_to_buf(elem->out_sg, elem->out_num, 0, &cpkt, sizeof(cpkt));
+        if (len == sizeof(cpkt)) {
+            handle_control_message(vser, &cpkt);
         }
-        iov_to_buf(elem->out_sg, elem->out_num, 0, buf, cur_len);
 
-        handle_control_message(vser, buf, cur_len);
         virtqueue_push(vq, elem, 0);
         g_free(elem);
     }
-    g_free(buf);
     virtio_notify(vdev, vq);
 }
 
-- 
2.54.0

Re: [PATCH] hw/char/virtio-serial-bus: fix guest-triggerable OOM in control_out()

[Daniel P. Berrangé]

On Mon, Jun 22, 2026 at 06:11:44PM +0200, Laurent Vivier wrote:
> A malicious guest can craft virtqueue descriptors with arbitrary lengths.
> control_out() calls iov_size() on the guest-supplied scatter-gather list
> and passes the result directly to g_malloc(), allowing a guest to force
> QEMU to attempt multi-gigabyte allocations and crash the host process.
> 
> Fix this by copying at most sizeof(struct virtio_console_control) into a
> stack-local variable instead of allocating a buffer sized by the guest.
> handle_control_message() only accesses the fixed-size id, event, and
> value fields, so no data beyond the struct was ever needed.

Does anyone have thoughts on whether we should treat guest initiated
unbounded allocs as a security issue ?

IIUC, this flaw would require root in the guest OS in order to craft
the malicious virtqueue descriptors.

A self-initiated crash triggered by root would not historically
be enough justification for CVE. We would require it to be triggered
by unprivileged user.

Nested virt with device assignment could change that equation though
as the L2 guest could be considered an unpriv user from the L1 POV.

Also in theory the large alloc might be large enough to consume all
host RAM but not large enough to trigger OOM kill of QEMU. This might
impact operation of other co-located VMs on the same host.

Anyone think this is bad enough to justify a CVE ? Or should we treat
these OOM scenarios maerely as "hardening" bugs, where they require
'root' in the L1 guest ?

> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3585
> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
> ---
>  hw/char/virtio-serial-bus.c | 34 +++++++---------------------------
>  1 file changed, 7 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
> index cd234dc6db1d..c1973f0248fc 100644
> --- a/hw/char/virtio-serial-bus.c
> +++ b/hw/char/virtio-serial-bus.c
> @@ -344,22 +344,16 @@ void virtio_serial_throttle_port(VirtIOSerialPort *port, bool throttle)
>  }
>  
>  /* Guest wants to notify us of some event */
> -static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
> +static void handle_control_message(VirtIOSerial *vser,
> +                                   struct virtio_console_control *gcpkt)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(vser);
>      struct VirtIOSerialPort *port;
>      VirtIOSerialPortClass *vsc;
> -    struct virtio_console_control cpkt, *gcpkt;
> +    struct virtio_console_control cpkt;
>      uint8_t *buffer;
>      size_t buffer_len;
>  
> -    gcpkt = buf;
> -
> -    if (len < sizeof(cpkt)) {
> -        /* The guest sent an invalid control packet */
> -        return;
> -    }
> -
>      cpkt.event = virtio_lduw_p(vdev, &gcpkt->event);
>      cpkt.value = virtio_lduw_p(vdev, &gcpkt->value);
>  
> @@ -457,41 +451,27 @@ static void control_in(VirtIODevice *vdev, VirtQueue *vq)
>  
>  static void control_out(VirtIODevice *vdev, VirtQueue *vq)
>  {
> +    struct virtio_console_control cpkt;
>      VirtQueueElement *elem;
>      VirtIOSerial *vser;
> -    uint8_t *buf;
>      size_t len;
>  
>      vser = VIRTIO_SERIAL(vdev);
>  
> -    len = 0;
> -    buf = NULL;
>      for (;;) {
> -        size_t cur_len;
> -
>          elem = virtqueue_pop(vq, sizeof(VirtQueueElement));
>          if (!elem) {
>              break;
>          }
>  
> -        cur_len = iov_size(elem->out_sg, elem->out_num);
> -        /*
> -         * Allocate a new buf only if we didn't have one previously or
> -         * if the size of the buf differs
> -         */
> -        if (cur_len > len) {
> -            g_free(buf);
> -
> -            buf = g_malloc(cur_len);
> -            len = cur_len;
> +        len = iov_to_buf(elem->out_sg, elem->out_num, 0, &cpkt, sizeof(cpkt));
> +        if (len == sizeof(cpkt)) {
> +            handle_control_message(vser, &cpkt);
>          }
> -        iov_to_buf(elem->out_sg, elem->out_num, 0, buf, cur_len);
>  
> -        handle_control_message(vser, buf, cur_len);
>          virtqueue_push(vq, elem, 0);
>          g_free(elem);
>      }
> -    g_free(buf);
>      virtio_notify(vdev, vq);
>  }
>  
> -- 
> 2.54.0
> 
> 

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