[Spice-commits] 2 commits - server/spicevmc.c
GitLab Mirror
gitlab-mirror at kemper.freedesktop.org
Tue Sep 24 12:14:47 UTC 2019
server/spicevmc.c | 44 +++++++++++++++++++++++++++++---------------
1 file changed, 29 insertions(+), 15 deletions(-)
New commits:
commit 1157588cc1e30ebfb17c27b744a658ca3e089bf4
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Sat Jun 1 12:10:17 2019 +0100
spicevmc: Avoids DoS if client is not able to get data faster enough
This fix half (one direction) of
https://gitlab.freedesktop.org/spice/spice/issues/29.
Specifically if you have attempt to transfer a file to the client
using WebDAV.
Previously the queue to the client was unbound. If client was not
getting data fast enough the server started queuing data.
To easily test this you can use a tool to limit bandwidth and
transfer a big file (I used a "dd if=/dev/zero bs=1M of=test") from
the guest.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Victor Toso <victortoso at redhat.com>
diff --git a/server/spicevmc.c b/server/spicevmc.c
index fea5be11..619f08cb 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -34,14 +34,14 @@
#include "reds.h"
#include "migration-protocol.h"
-/* todo: add flow control. i.e.,
- * (a) limit the tokens available for the client
- * (b) limit the tokens available for the server
- */
/* 64K should be enough for all but the largest writes + 32 bytes hdr */
#define BUF_SIZE (64 * 1024 + 32)
#define COMPRESS_THRESHOLD 1000
+// limit of the queued data, at this limit we stop reading from device to
+// avoid DoS
+#define QUEUED_DATA_LIMIT (1024*1024)
+
SPICE_DECLARE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, CHAR_DEVICE_SPICEVMC);
#define RED_TYPE_CHAR_DEVICE_SPICEVMC red_char_device_spicevmc_get_type()
@@ -83,6 +83,7 @@ struct RedCharDeviceSpiceVmcClass
static RedCharDevice *red_char_device_spicevmc_new(SpiceCharDeviceInstance *sin,
RedsState *reds,
RedVmcChannel *channel);
+static void spicevmc_red_channel_queue_data(RedVmcChannel *channel, RedVmcPipeItem *item);
G_DEFINE_TYPE(RedCharDeviceSpiceVmc, red_char_device_spicevmc, RED_TYPE_CHAR_DEVICE)
@@ -96,6 +97,7 @@ struct RedVmcChannel
RedVmcPipeItem *pipe_item;
RedCharDeviceWriteBuffer *recv_from_client_buf;
uint8_t port_opened;
+ uint32_t queued_data;
RedStatCounter in_data;
RedStatCounter in_compressed;
RedStatCounter in_decompressed;
@@ -337,7 +339,7 @@ static RedPipeItem *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
sif = spice_char_device_get_interface(sin);
- if (!channel->rcc) {
+ if (!channel->rcc || channel->queued_data >= QUEUED_DATA_LIMIT) {
return NULL;
}
@@ -360,14 +362,14 @@ static RedPipeItem *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
msg_item_compressed = try_compress_lz4(channel, n, msg_item);
if (msg_item_compressed != NULL) {
- red_channel_client_pipe_add_push(channel->rcc, &msg_item_compressed->base);
+ spicevmc_red_channel_queue_data(channel, msg_item_compressed);
return NULL;
}
#endif
stat_inc_counter(channel->out_data, n);
msg_item->uncompressed_data_size = n;
msg_item->buf_used = n;
- red_channel_client_pipe_add_push(channel->rcc, &msg_item->base);
+ spicevmc_red_channel_queue_data(channel, msg_item);
return NULL;
}
channel->pipe_item = msg_item;
@@ -609,11 +611,19 @@ static void spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
}
}
+static void
+spicevmc_red_channel_queue_data(RedVmcChannel *channel, RedVmcPipeItem *item)
+{
+ channel->queued_data += item->buf_used;
+ red_channel_client_pipe_add_push(channel->rcc, &item->base);
+}
+
static void spicevmc_red_channel_send_data(RedChannelClient *rcc,
SpiceMarshaller *m,
RedPipeItem *item)
{
RedVmcPipeItem *i = SPICE_UPCAST(RedVmcPipeItem, item);
+ RedVmcChannel *channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
/* for compatibility send using not compressed data message */
if (i->type == SPICE_DATA_COMPRESSION_TYPE_NONE) {
@@ -630,6 +640,14 @@ static void spicevmc_red_channel_send_data(RedChannelClient *rcc,
red_pipe_item_ref(item);
spice_marshaller_add_by_ref_full(m, i->buf, i->buf_used,
marshaller_unref_pipe_item, item);
+
+ // account for sent data and wake up device if was blocked
+ uint32_t old_queued_data = channel->queued_data;
+ channel->queued_data -= i->buf_used;
+ if (channel->chardev &&
+ old_queued_data >= QUEUED_DATA_LIMIT && channel->queued_data < QUEUED_DATA_LIMIT) {
+ red_char_device_wakeup(channel->chardev);
+ }
}
static void spicevmc_red_channel_send_migrate_data(RedChannelClient *rcc,
@@ -768,6 +786,7 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
return;
}
vmc_channel->rcc = rcc;
+ vmc_channel->queued_data = 0;
red_channel_client_ack_zero_messages_window(rcc);
if (strcmp(sin->subtype, "port") == 0) {
commit 540f70351dcf3b3de22d27354394cd8f3ca832b5
Author: Frediano Ziglio <fziglio at redhat.com>
Date: Sat Jun 1 11:47:11 2019 +0100
spicevmc: Do not use RedCharDevice pipe items handling
As we don't use any token there's no reason to not queue directly instead
of passing through RedCharDevice.
This will make easier to limit the queue which currently is unlimited.
RedCharDevice flow control has some problems:
- it's really designed with VDI in mind. This for SpiceVMC would cause
code implementation to be confusing having to satisfy the agent token
handling;
- it's using items as unit not allowing counting bytes;
- it duplicates some queue management between RedChannelClient;
- it's broken (see comments in char-device.h);
- it forces "clients" to behave in some way not altering for instance the
queued items (which is very useful if items can be collapsed together);
- it replicates the token handling on the device queue too. This could
seems right but is not that if you have a network card going @ 1 GBit/s
you are able to upload more than 1 Gbit/s just using multiple
connections. The kernel will use a single queue for the network interface
limiting and sharing de facto the various buffers between the multiple
connections.
Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
Acked-by: Victor Toso <victortoso at redhat.com>
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 546fe774..fea5be11 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -360,29 +360,24 @@ static RedPipeItem *spicevmc_chardev_read_msg_from_dev(RedCharDevice *self,
msg_item_compressed = try_compress_lz4(channel, n, msg_item);
if (msg_item_compressed != NULL) {
- return &msg_item_compressed->base;
+ red_channel_client_pipe_add_push(channel->rcc, &msg_item_compressed->base);
+ return NULL;
}
#endif
stat_inc_counter(channel->out_data, n);
msg_item->uncompressed_data_size = n;
msg_item->buf_used = n;
- return &msg_item->base;
- } else {
- channel->pipe_item = msg_item;
+ red_channel_client_pipe_add_push(channel->rcc, &msg_item->base);
return NULL;
}
+ channel->pipe_item = msg_item;
+ return NULL;
}
static void spicevmc_chardev_send_msg_to_client(RedCharDevice *self,
RedPipeItem *msg,
RedClient *client)
{
- RedCharDeviceSpiceVmc *vmc = RED_CHAR_DEVICE_SPICEVMC(self);
- RedVmcChannel *channel = RED_VMC_CHANNEL(vmc->channel);
-
- spice_assert(red_channel_client_get_client(channel->rcc) == client);
- red_pipe_item_ref(msg);
- red_channel_client_pipe_add_push(channel->rcc, msg);
}
static void red_port_init_item_free(struct RedPipeItem *base)
More information about the Spice-commits
mailing list