On Mon, Jun 22, 2026 at 6:30 PM Daniel P. Berrangé <[email protected]> wrote: > > 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 ?
I'd lean toward classifying these as hardening bugs. I don't see the point in assigning Low CVEs to these kinds of issues nowadays. Under current vulnerability management standards, they would definitely be pushed down the priority list and likely skipped or deferred. > > Cc: [email protected] > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3585 > > Signed-off-by: Laurent Vivier <[email protected]> > > --- > > 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 :| > -- Mauro Matteo Cascella Red Hat Product Security PGP-Key ID: BB3410B0
