[Spice-commits] 2 commits - configure.ac server/stream-device.c server/tests

[Frediano Ziglio]

 configure.ac                      |    2 +-
 server/stream-device.c            |   32 +++++++++++++++++++++++---------
 server/tests/test-stream-device.c |   11 ++++++++---
 3 files changed, 32 insertions(+), 13 deletions(-)

New commits:
commit 137d9ec8e9aa9b3e0aadbb5b51ca098ca22ec7db
Author: Frediano Ziglio <fziglio at redhat.com>
Date:   Thu Nov 23 12:45:01 2017 +0000

    test-stream-device: Check incomplete reads of StreamMsgFormat
    
    Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
    Acked-by: Jonathon Jongsma <jjongsma at redhat.com>

diff --git a/server/tests/test-stream-device.c b/server/tests/test-stream-device.c
index c6f47758..656bf56b 100644
--- a/server/tests/test-stream-device.c
+++ b/server/tests/test-stream-device.c
@@ -139,6 +139,11 @@ static void test_stream_device(void)
     ++message_sizes_end;
 
     p = add_format(p, 640, 480, SPICE_VIDEO_CODEC_TYPE_VP9);
+
+    // this split the second format in half
+    *message_sizes_end = p - message - 4;
+    ++message_sizes_end;
+
     *message_sizes_end = p - message;
     ++message_sizes_end;
 
@@ -163,10 +168,10 @@ static void test_stream_device(void)
     spice_server_port_event(&vmc_instance, SPICE_PORT_EVENT_OPENED);
     spice_server_char_device_wakeup(&vmc_instance);
 
-    // make sure first 2 parts are read completely
-    g_assert(message_sizes_curr - message_sizes >= 2);
+    // make sure first 3 parts are read completely
+    g_assert(message_sizes_curr - message_sizes >= 3);
     // make sure last part is not read at all
-    g_assert(message_sizes_curr - message_sizes < 4);
+    g_assert(message_sizes_curr - message_sizes < 5);
 
     test_destroy(test);
     basic_event_loop_destroy();
commit 61aa6ac8aa150392c849ac76e2d226973d57ff0f
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Wed Nov 15 14:04:33 2017 -0600

    StreamDevice: Handle incomplete reads of StreamMsgFormat
    
    This is currently unlikely to happen since we communicate over a pipe
    and the pipe buffer is sufficiently large to avoid splitting the
    message. But for completeness, we should handle this scenario.
    
    Signed-off-by: Jonathon Jongsma <jjongsma at redhat.com>
    Acked-by: Frediano Ziglio <fziglio at redhat.com>

diff --git a/configure.ac b/configure.ac
index fb266ad4..3401dba8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -156,7 +156,7 @@ AS_IF([test x"$have_smartcard" = "xyes"], [
     AS_VAR_APPEND([SPICE_REQUIRES], [" libcacard >= 0.1.2"])
 ])
 
-SPICE_PROTOCOL_MIN_VER=0.12.13
+SPICE_PROTOCOL_MIN_VER=0.12.14
 PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= $SPICE_PROTOCOL_MIN_VER])
 AC_SUBST([SPICE_PROTOCOL_MIN_VER])
 
diff --git a/server/stream-device.c b/server/stream-device.c
index fc5b5065..0953a6d0 100644
--- a/server/stream-device.c
+++ b/server/stream-device.c
@@ -42,6 +42,12 @@ struct StreamDevice {
 
     StreamDevHeader hdr;
     uint8_t hdr_pos;
+    union {
+        StreamMsgFormat format;
+        StreamMsgCapabilities capabilities;
+        uint8_t buf[STREAM_MSG_CAPABILITIES_MAX_BYTES];
+    } msg;
+    uint32_t msg_pos;
     bool has_error;
     bool opened;
     bool flow_stopped;
@@ -89,6 +95,7 @@ stream_device_read_msg_from_dev(RedCharDevice *self, SpiceCharDeviceInstance *si
         if (dev->hdr_pos >= sizeof(dev->hdr)) {
             dev->hdr.type = GUINT16_FROM_LE(dev->hdr.type);
             dev->hdr.size = GUINT32_FROM_LE(dev->hdr.size);
+            dev->msg_pos = 0;
         }
     }
 
@@ -155,19 +162,25 @@ handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin, const char *
 static bool
 handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin)
 {
-    StreamMsgFormat fmt;
     SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
-    int n = sif->read(sin, (uint8_t *) &fmt, sizeof(fmt));
-    if (n == 0) {
-        return false;
-    }
-    if (n != sizeof(fmt)) {
+
+    spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
+    spice_assert(dev->hdr.type == STREAM_TYPE_FORMAT);
+
+    int n = sif->read(sin, dev->msg.buf + dev->msg_pos, sizeof(StreamMsgFormat) - dev->msg_pos);
+    if (n < 0) {
         return handle_msg_invalid(dev, sin, NULL);
     }
-    fmt.width = GUINT32_FROM_LE(fmt.width);
-    fmt.height = GUINT32_FROM_LE(fmt.height);
-    stream_channel_change_format(dev->stream_channel, &fmt);
 
+    dev->msg_pos += n;
+
+    if (dev->msg_pos < sizeof(StreamMsgFormat)) {
+        return false;
+    }
+
+    dev->msg.format.width = GUINT32_FROM_LE(dev->msg.format.width);
+    dev->msg.format.height = GUINT32_FROM_LE(dev->msg.format.height);
+    stream_channel_change_format(dev->stream_channel, &dev->msg.format);
     return true;
 }
 
@@ -334,6 +347,7 @@ stream_device_port_event(RedCharDevice *char_dev, uint8_t event)
         allocate_channels(dev);
     }
     dev->hdr_pos = 0;
+    dev->msg_pos = 0;
     dev->has_error = false;
     dev->flow_stopped = false;
     red_char_device_reset(char_dev);