[Qemu-devel] [PATCH v2 0/4] virtio-serial: Trivial fixes, don't copy buffers to host

[Qemu-devel] [PATCH v2 0/4] virtio-serial: Trivial fixes, don't copy buffers to host

[Amit Shah]

Hi,

This patch series converts virtio-serial-bus to use the guest buffers
instead of copying over guest data to the host, as suggested by Paul.

In addition, there are some trivial fixes for the virtio-console and
virtio-serial code.

v2:
 - drop the erroring out patch till we decide what's to be done
 - remove goto usage.

Amit Shah (4):
  virtio-console: Factor out common init between console and generic
    ports
  virtio-console: Remove unnecessary braces
  virtio-serial: Simplify condition for a while loop
  virtio-serial: Don't copy over guest buffer to host

 hw/virtio-console.c    |   34 +++++++++++++++-------------------
 hw/virtio-serial-bus.c |   21 ++++++++++++---------
 2 files changed, 27 insertions(+), 28 deletions(-)

-- 
1.7.3.2

[Qemu-devel] [PATCH v2 1/4] virtio-console: Factor out common init between console and generic ports

[Amit Shah]

The initialisation for generic ports and console ports is similar.
Factor out the parts that are the same in a different function that can
be called from each of the initfns.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |   31 ++++++++++++++-----------------
 1 files changed, 14 insertions(+), 17 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index caea11f..d7fe68b 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -58,24 +58,28 @@ static void chr_event(void *opaque, int event)
     }
 }
 
-/* Virtio Console Ports */
-static int virtconsole_initfn(VirtIOSerialDevice *dev)
+static int generic_port_init(VirtConsole *vcon, VirtIOSerialDevice *dev)
 {
-    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
-    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
-
-    port->info = dev->info;
-
-    port->is_console = true;
+    vcon->port.info = dev->info;
 
     if (vcon->chr) {
         qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
                               vcon);
-        port->info->have_data = flush_buf;
+        vcon->port.info->have_data = flush_buf;
     }
     return 0;
 }
 
+/* Virtio Console Ports */
+static int virtconsole_initfn(VirtIOSerialDevice *dev)
+{
+    VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
+    VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
+
+    port->is_console = true;
+    return generic_port_init(vcon, dev);
+}
+
 static int virtconsole_exitfn(VirtIOSerialDevice *dev)
 {
     VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
@@ -115,14 +119,7 @@ static int virtserialport_initfn(VirtIOSerialDevice *dev)
     VirtIOSerialPort *port = DO_UPCAST(VirtIOSerialPort, dev, &dev->qdev);
     VirtConsole *vcon = DO_UPCAST(VirtConsole, port, port);
 
-    port->info = dev->info;
-
-    if (vcon->chr) {
-        qemu_chr_add_handlers(vcon->chr, chr_can_read, chr_read, chr_event,
-                              vcon);
-        port->info->have_data = flush_buf;
-    }
-    return 0;
+    return generic_port_init(vcon, dev);
 }
 
 static VirtIOSerialPortInfo virtserialport_info = {
-- 
1.7.3.2

[Qemu-devel] [PATCH v2 2/4] virtio-console: Remove unnecessary braces

[Amit Shah]

Remove unnecessary braces around a case statement.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-console.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-console.c b/hw/virtio-console.c
index d7fe68b..d0b9354 100644
--- a/hw/virtio-console.c
+++ b/hw/virtio-console.c
@@ -48,10 +48,9 @@ static void chr_event(void *opaque, int event)
     VirtConsole *vcon = opaque;
 
     switch (event) {
-    case CHR_EVENT_OPENED: {
+    case CHR_EVENT_OPENED:
         virtio_serial_open(&vcon->port);
         break;
-    }
     case CHR_EVENT_CLOSED:
         virtio_serial_close(&vcon->port);
         break;
-- 
1.7.3.2

[Qemu-devel] [PATCH v2 3/4] virtio-serial: Simplify condition for a while loop

[Amit Shah]

Separate out a non-changing condition over the period of a loop into an
if statement before the loop.  This will be used later to re-work the
loop.

Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index 74ba5ec..ecf0056 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -121,7 +121,10 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
     assert(port || discard);
     assert(virtio_queue_ready(vq));
 
-    while ((discard || !port->throttled) && virtqueue_pop(vq, &elem)) {
+    if (!discard && port->throttled) {
+        return;
+    }
+    while (virtqueue_pop(vq, &elem)) {
         uint8_t *buf;
         size_t ret, buf_size;
 
-- 
1.7.3.2

[Qemu-devel] [PATCH v2 4/4] virtio-serial: Don't copy over guest buffer to host

[Amit Shah]

When the guest writes something to a host, we copied over the entire
buffer first into the host and then processed it.  Do away with that, it
could result in a malicious guest causing a DoS on the host.

Reported-by: Paul Brook <paul@codesourcery.com>
Signed-off-by: Amit Shah <amit.shah@redhat.com>
---
 hw/virtio-serial-bus.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/virtio-serial-bus.c b/hw/virtio-serial-bus.c
index ecf0056..a0886a2 100644
--- a/hw/virtio-serial-bus.c
+++ b/hw/virtio-serial-bus.c
@@ -125,16 +125,16 @@ static void do_flush_queued_data(VirtIOSerialPort *port, VirtQueue *vq,
         return;
     }
     while (virtqueue_pop(vq, &elem)) {
-        uint8_t *buf;
-        size_t ret, buf_size;
+        unsigned int i;
 
-        if (!discard) {
-            buf_size = iov_size(elem.out_sg, elem.out_num);
-            buf = qemu_malloc(buf_size);
-            ret = iov_to_buf(elem.out_sg, elem.out_num, buf, 0, buf_size);
+        for (i = 0; !discard && i < elem.out_num; i++) {
+            size_t buf_size;
 
-            port->info->have_data(port, buf, ret);
-            qemu_free(buf);
+            buf_size = elem.out_sg[i].iov_len;
+
+            port->info->have_data(port,
+                                  elem.out_sg[i].iov_base,
+                                  buf_size);
         }
         virtqueue_push(vq, &elem, 0);
     }
-- 
1.7.3.2