[Spice-devel] [spice-server v1] char-device: fix mismatch of client tokens

[Victor Toso]

As the tokens counter were not being reset you could enter in a
situation where client thinks it has more tokens then server which
would eventually lead to client's disconnection from 0c5eca97f16ec6
onwards (before it was crashing).

It is easy to check the above situation if you track the amount of
tokens you have in the client and simply kill and restart the agent
while doing some file transfer: the client could reach more then 13
tokens which should not really be possible.

Based on patch from Frediano Ziglio <fziglio at redhat.com>
---
 server/char-device.c |  6 ++++++
 server/reds.c        | 17 +++++------------
 2 files changed, 11 insertions(+), 12 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index cb35aa2..957fb26 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -851,6 +851,12 @@ void red_char_device_reset(RedCharDevice *dev)
         dev_client->num_send_tokens += g_queue_get_length(dev_client->send_queue);
         g_queue_foreach(dev_client->send_queue, (GFunc)red_pipe_item_unref, NULL);
         g_queue_clear(dev_client->send_queue);
+
+        /* If device is reset, we must reset the tokens counters as well as we
+         * don't hold any data from client and upon agent's reconnection we send
+         * SPICE_MSG_MAIN_AGENT_CONNECTED_TOKENS with all free tokens we have */
+        dev_client->num_client_tokens += dev_client->num_client_tokens_free;
+        dev_client->num_client_tokens_free = 0;
     }
     red_char_device_reset_dev_instance(dev, NULL);
 }
diff --git a/server/reds.c b/server/reds.c
index 4fd1d35..27a5957 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -542,23 +542,16 @@ static void reds_reset_vdp(RedsState *reds)
     dev->priv->write_filter.discard_all = TRUE;
     dev->priv->client_agent_started = FALSE;
 
-    /* resetting and not destroying the dev as a workaround for a bad
-     * tokens management in the vdagent protocol:
-     *  The client tokens' are set only once, when the main channel is initialized.
-     *  Instead, it would have been more appropriate to reset them upon AGENT_CONNECT.
+    /*  The client's tokens are set once when the main channel is initialized
+     *  and once upon agent's connection with SPICE_MSG_MAIN_AGENT_CONNECTED_TOKENS.
      *  The client tokens are tracked as part of the RedCharDeviceClient. Thus,
      *  in order to be backward compatible with the client, we need to track the tokens
      *  even if the agent is detached. We don't destroy the char_device, and
      *  instead we just reset it.
-     *  In addition, there used to be a misshandling of AGENT_TOKENS message in spice-gtk: it
-     *  overrides the amount of tokens, instead of adding the given amount.
+     *  The tokens are also reset to avoid mismatch in upon agent reconnection.
      */
-    if (red_channel_test_remote_cap(&reds->main_channel->base,
-                                    SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS)) {
-        dev->priv->agent_attached = FALSE;
-    } else {
-        red_char_device_reset(RED_CHAR_DEVICE(dev));
-    }
+    dev->priv->agent_attached = FALSE;
+    red_char_device_reset(RED_CHAR_DEVICE(dev));
 
     sif = spice_char_device_get_interface(reds->vdagent);
     if (sif->state) {
-- 
2.5.5