[spice-server,4/4] spicevmc: Avoids DoS if guest device is not able to get data faster enough

Submitted by Frediano Ziglio on June 17, 2019, 3:40 p.m.

Details

Message ID 20190617154011.20310-4-fziglio@redhat.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio June 17, 2019, 3:40 p.m.
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file from the client
using WebDAV.
Previously the queue to the device was unbound. If device was not
getting data fast enough the server started queuing data.
To easily test this you can suspend the WebDAV daemon while transferring
a big file from the client.
To simplify the code and reduce the changes server buffers are
used. This as client token implementation is written with agent
in mind. While when we exhaust server tokens RedCharDevice doesn't
close the client when we exhaust client tokens RedCharDevice closes
the client which in this case it's not wanted.

Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
---
 server/spicevmc.c | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/spicevmc.c b/server/spicevmc.c
index 02e90199c..1209e7155 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -491,9 +491,9 @@  static bool handle_compressed_msg(RedVmcChannel *channel, RedChannelClient *rcc,
     int decompressed_size;
     RedCharDeviceWriteBuffer *write_buf;
 
-    write_buf = red_char_device_write_buffer_get_client(channel->chardev,
-                                                        red_channel_client_get_client(rcc),
-                                                        compressed_data_msg->uncompressed_size);
+    write_buf = red_char_device_write_buffer_get_server(channel->chardev,
+                                                        compressed_data_msg->uncompressed_size,
+                                                        false);
     if (!write_buf) {
         return FALSE;
     }
@@ -565,6 +565,15 @@  static bool spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
     return TRUE;
 }
 
+/* if device manage to send some data attempt to unblock the channel */
+static void spicevmc_on_free_self_token(RedCharDevice *self)
+{
+    RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
+    RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
+
+    red_channel_client_unblock_read(channel->rcc);
+}
+
 static uint8_t *spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
                                                        uint16_t type,
                                                        uint32_t size)
@@ -572,16 +581,14 @@  static uint8_t *spicevmc_red_channel_alloc_msg_rcv_buf(RedChannelClient *rcc,
 
     switch (type) {
     case SPICE_MSGC_SPICEVMC_DATA: {
-        RedClient *client = red_channel_client_get_client(rcc);
         RedVmcChannel *channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
 
         assert(!channel->recv_from_client_buf);
 
-        channel->recv_from_client_buf = red_char_device_write_buffer_get_client(channel->chardev,
-                                                                                client,
-                                                                                size);
+        channel->recv_from_client_buf = red_char_device_write_buffer_get_server(channel->chardev,
+                                                                                size, true);
         if (!channel->recv_from_client_buf) {
-            spice_error("failed to allocate write buffer");
+            red_channel_client_block_read(rcc);
             return NULL;
         }
         return channel->recv_from_client_buf->buf;
@@ -908,6 +915,7 @@  red_char_device_spicevmc_class_init(RedCharDeviceSpiceVmcClass *klass)
     char_dev_class->send_msg_to_client = spicevmc_chardev_send_msg_to_client;
     char_dev_class->remove_client = spicevmc_char_dev_remove_client;
     char_dev_class->port_event = spicevmc_port_event;
+    char_dev_class->on_free_self_token = spicevmc_on_free_self_token;
 
     g_object_class_install_property(object_class,
                                     PROP_CHANNEL,
@@ -934,7 +942,7 @@  red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
                         "sin", sin,
                         "spice-server", reds,
                         "client-tokens-interval", 0ULL,
-                        "self-tokens", ~0ULL,
+                        "self-tokens", 128, // limit number of messages sent to device
                         "channel", channel,
                         NULL);
 }