[Spice-devel] [PATCH spice-server v2] StreamDevice: Handle incomplete reads of StreamMsgFormat

[Jonathon Jongsma]

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>
---
Since v1:
 - updates from Frediano's review
 - bumps spice-protocol requirement for
   STREAM_MSG_CAPABILITIES_MAX_BYTES symbol
 - change msg_pos from uint8_t to uint32_t
 - move reset of msg_pos to calling function
   (stream_device_port_event())

 configure.ac           |  2 +-
 server/stream-device.c | 31 ++++++++++++++++++++++---------
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/configure.ac b/configure.ac
index fb266ad4c..3401dba83 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 fc5b50659..efa6d8db5 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;
@@ -155,19 +161,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 +346,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);
-- 
2.13.6