[PATCH 4/9] virtio-serial: Handle scatter-gather buffers for control messages

[Amit Shah <amit.shah@xxxxxxxxxx>]

Current control messages are small enough to not be split into multiple
buffers but we could run into such a situation in the future or a
malicious guest could cause such a situation.

So handle the entire iov request for control messages.

Also ensure the size of the control request is >= what we expect
otherwise we risk accessing memory that we don't own.

Signed-off-by: Amit Shah <amit.shah@xxxxxxxxxx>
CC: Avi Kivity <avi@xxxxxxxxxx>
Reported-by: Avi Kivity <avi@xxxxxxxxxx>
---
 hw/virtio-serial-bus.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 830eb75..d5887ab 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -200,7 +200,7 @@ size_t virtio_serial_guest_ready(VirtIOSerialPort *port)
 }
 
 /* Guest wants to notify us of some event */
-static void handle_control_message(VirtIOSerial *vser, void *buf)
+static void handle_control_message(VirtIOSerial *vser, void *buf, size_t len)
 {
     struct VirtIOSerialPort *port;
     struct virtio_console_control cpkt, *gcpkt;
@@ -208,6 +208,10 @@ static void handle_control_message(VirtIOSerial *vser, void *buf)
     size_t buffer_len;
 
     gcpkt = buf;
+    if (len < sizeof(cpkt)) {
+        /* The guest sent an invalid control packet */
+        return;
+    }
     port = find_port_by_id(vser, ldl_p(&gcpkt->id));
     if (!port)
         return;
@@ -281,12 +285,45 @@ static void control_out(VirtIODevice *vdev, VirtQueue *vq)
 {
     VirtQueueElement elem;
     VirtIOSerial *vser;
+    uint8_t *buf;
+    size_t len;
 
     vser = DO_UPCAST(VirtIOSerial, vdev, vdev);
 
+    len = 0;
+    buf = NULL;
     while (virtqueue_pop(vq, &elem)) {
-        handle_control_message(vser, elem.out_sg[0].iov_base);
-        virtqueue_push(vq, &elem, elem.out_sg[0].iov_len);
+        unsigned int i;
+        size_t cur_len, offset;
+
+        cur_len = 0;
+        for (i = 0; i < elem.out_num; i++) {
+            cur_len += elem.out_sg[i].iov_len;
+        }
+        /*
+         * Allocate a new buf only if we didn't have one previously or
+         * if the size of the buf differs
+         */
+        if (cur_len != len) {
+            if (len) {
+                qemu_free(buf);
+            }
+            buf = qemu_malloc(cur_len);
+        }
+
+        offset = 0;
+        for (i = 0; i < elem.out_num; i++) {
+            memcpy(buf + offset, elem.out_sg[i].iov_base,
+                   elem.out_sg[i].iov_len);
+            offset += elem.out_sg[i].iov_len;
+        }
+        len = cur_len;
+
+        handle_control_message(vser, buf, len);
+        virtqueue_push(vq, &elem, len);
+    }
+    if (len) {
+        qemu_free(buf);
     }
     virtio_notify(vdev, vq);
 }
-- 
1.6.2.5

_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/virtualization