[Spice-devel] [PATCH spice-server v2 4/7] smartcard: Fix parsing multiple messages from the device

[Frediano Ziglio]

If the server is busy or the guest write multiple requests with
a single operation it could happen that we receive multiple
requests with a single read.
This will make "remaining" > 0.
Use memmove instead of memcpy as the buffer can overlap if the
second request if bigger than the first.
"buf_pos" points to the point of the buffer after we read, if
we want the first part of the next request is "buf_pos - remaining".
Same consideration setting "buf_pos" for the next iteration.
Also check the loop condition. If the remaining buffer contains
a full request we don't need to read other bytes (note that there
could be no bytes left), just parse the request.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/smartcard.c | 27 ++++++++++++++++++---------
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/server/smartcard.c b/server/smartcard.c
index 4c5bba07d..340118e18 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -130,19 +130,28 @@ static RedPipeItem *smartcard_read_msg_from_device(RedCharDevice *self,
     RedCharDeviceSmartcard *dev = RED_CHAR_DEVICE_SMARTCARD(self);
     SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin);
     VSCMsgHeader *vheader = (VSCMsgHeader*)dev->priv->buf;
-    int n;
     int remaining;
     int actual_length;
 
-    while ((n = sif->read(sin, dev->priv->buf_pos, dev->priv->buf_size - dev->priv->buf_used)) > 0) {
+    while (true) {
         RedMsgItem *msg_to_client;
 
-        dev->priv->buf_pos += n;
-        dev->priv->buf_used += n;
-        if (dev->priv->buf_used < sizeof(VSCMsgHeader)) {
-            continue;
+        // it's possible we already got a full message from a previous partial
+        // read. In this case we don't need to read any byte
+        if (dev->priv->buf_used < sizeof(VSCMsgHeader) ||
+            dev->priv->buf_used - sizeof(VSCMsgHeader) < ntohl(vheader->length)) {
+            int n = sif->read(sin, dev->priv->buf_pos, dev->priv->buf_size - dev->priv->buf_used);
+            if (n <= 0) {
+                break;
+            }
+            dev->priv->buf_pos += n;
+            dev->priv->buf_used += n;
+
+            if (dev->priv->buf_used < sizeof(VSCMsgHeader)) {
+                continue;
+            }
+            smartcard_read_buf_prepare(dev, vheader);
         }
-        smartcard_read_buf_prepare(dev, vheader);
         actual_length = ntohl(vheader->length);
         if (dev->priv->buf_used - sizeof(VSCMsgHeader) < actual_length) {
             continue;
@@ -150,9 +159,9 @@ static RedPipeItem *smartcard_read_msg_from_device(RedCharDevice *self,
         msg_to_client = smartcard_char_device_on_message_from_device(dev, vheader);
         remaining = dev->priv->buf_used - sizeof(VSCMsgHeader) - actual_length;
         if (remaining > 0) {
-            memcpy(dev->priv->buf, dev->priv->buf_pos, remaining);
+            memmove(dev->priv->buf, dev->priv->buf_pos - remaining, remaining);
         }
-        dev->priv->buf_pos = dev->priv->buf;
+        dev->priv->buf_pos = dev->priv->buf + remaining;
         dev->priv->buf_used = remaining;
         if (msg_to_client) {
             return &msg_to_client->base;
-- 
2.21.0