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

[Victor Toso]

Hi,

On Mon, Jun 17, 2019 at 04:40:11PM +0100, Frediano Ziglio wrote:
> 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 at redhat.com>

Tested a bit with Fedora guest + shared folder, looks fine. For
the series,
Acked-by: Victor Toso <victortoso at redhat.com>

> ---
>  server/spicevmc.c | 26 +++++++++++++++++---------
>  1 file changed, 17 insertions(+), 9 deletions(-)
> 
> 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);
>  }
> -- 
> 2.20.1
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20190924/d96f93e7/attachment.sig>