diff --git a/.gitignore b/.gitignore index 36fc37b..5b72b89 100644 --- a/.gitignore +++ b/.gitignore @@ -17,3 +17,4 @@ spice-0.5.3.tar.bz2 /spice-0.12.2.tar.bz2 /spice-0.12.3.tar.bz2 /spice-0.12.4.tar.bz2 +/spice-0.12.5.tar.bz2 diff --git a/0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch b/0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch deleted file mode 100644 index aab0d8e..0000000 --- a/0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch +++ /dev/null @@ -1,96 +0,0 @@ -From 47e722b85ccd0b6876ca189a3d6f6f05289fe3c3 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Wed, 24 Jul 2013 14:54:23 -0400 -Subject: [PATCH 1/8] red_channel: prevent adding and pushing pipe items after - a channel_client has diconnected - -Fixes: leaks of pipe items & "red_client_destroy: assertion `rcc->send_data.size == 0'" - -red_channel_disconnect clears the pipe. It is called only once. After, -it was called, not items should be added to the pipe. - -An example of when this assert can occur: -on_new_cursor_channel (red_worker.c), pushes 2 pipe items. -When it pushes the first pipe item, if the client has disconnected, -it can hit a socket error, and then, red_channel_client_disconnect is called. -The second call to adding a pipe item, will add the item to -the pipe. red_channel_client_pipe_add_type also calls -red_channel_client_push, which will update the send_data.size. -Then, the push will also hit a socket error, but red_channel_client_disconnect -won't clear the pending pipe item again, since it was already called. -When red_client_destory is called, we hit assertion `rcc->send_data.size -== 0'. -Note that if a pipe item is added to the pipe after -red_channel_client_disconnect was called, but without pushing it, -we should hit "spice_assert(rcc->pipe_size == 0)". ---- - server/red_channel.c | 30 ++++++++++++++++++++++++------ - 1 file changed, 24 insertions(+), 6 deletions(-) - -diff --git a/server/red_channel.c b/server/red_channel.c -index 33af388..0d74413 100644 ---- a/server/red_channel.c -+++ b/server/red_channel.c -@@ -1527,9 +1527,23 @@ void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type) - item->type = type; - } - --void red_channel_client_pipe_add(RedChannelClient *rcc, PipeItem *item) -+static inline int validate_pipe_add(RedChannelClient *rcc, PipeItem *item) - { - spice_assert(rcc && item); -+ if (SPICE_UNLIKELY(!red_channel_client_is_connected(rcc))) { -+ spice_debug("rcc is disconnected %p", rcc); -+ red_channel_client_release_item(rcc, item, FALSE); -+ return FALSE; -+ } -+ return TRUE; -+} -+ -+void red_channel_client_pipe_add(RedChannelClient *rcc, PipeItem *item) -+{ -+ -+ if (!validate_pipe_add(rcc, item)) { -+ return; -+ } - rcc->pipe_size++; - ring_add(&rcc->pipe, &item->link); - } -@@ -1543,10 +1557,10 @@ void red_channel_client_pipe_add_push(RedChannelClient *rcc, PipeItem *item) - void red_channel_client_pipe_add_after(RedChannelClient *rcc, - PipeItem *item, PipeItem *pos) - { -- spice_assert(rcc); - spice_assert(pos); -- spice_assert(item); -- -+ if (!validate_pipe_add(rcc, item)) { -+ return; -+ } - rcc->pipe_size++; - ring_add_after(&item->link, &pos->link); - } -@@ -1560,14 +1574,18 @@ int red_channel_client_pipe_item_is_linked(RedChannelClient *rcc, - void red_channel_client_pipe_add_tail_no_push(RedChannelClient *rcc, - PipeItem *item) - { -- spice_assert(rcc); -+ if (!validate_pipe_add(rcc, item)) { -+ return; -+ } - rcc->pipe_size++; - ring_add_before(&item->link, &rcc->pipe); - } - - void red_channel_client_pipe_add_tail(RedChannelClient *rcc, PipeItem *item) - { -- spice_assert(rcc); -+ if (!validate_pipe_add(rcc, item)) { -+ return; -+ } - rcc->pipe_size++; - ring_add_before(&item->link, &rcc->pipe); - red_channel_client_push(rcc); --- -1.8.3.1 - diff --git a/0002-red_channel-add-ref-count-to-RedClient.patch b/0002-red_channel-add-ref-count-to-RedClient.patch deleted file mode 100644 index 10e24b9..0000000 --- a/0002-red_channel-add-ref-count-to-RedClient.patch +++ /dev/null @@ -1,96 +0,0 @@ -From aab45618cc12799d5f7351ef8832ae73b33057c7 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Fri, 26 Jul 2013 13:45:16 -0400 -Subject: [PATCH 2/8] red_channel: add ref count to RedClient - ---- - server/red_channel.c | 23 ++++++++++++++++++++--- - server/red_channel.h | 17 +++++++++++++++-- - 2 files changed, 35 insertions(+), 5 deletions(-) - -diff --git a/server/red_channel.c b/server/red_channel.c -index 0d74413..9168b8a 100644 ---- a/server/red_channel.c -+++ b/server/red_channel.c -@@ -1932,10 +1932,29 @@ RedClient *red_client_new(int migrated) - pthread_mutex_init(&client->lock, NULL); - client->thread_id = pthread_self(); - client->during_target_migrate = migrated; -+ client->refs = 1; - - return client; - } - -+RedClient *red_client_ref(RedClient *client) -+{ -+ spice_assert(client); -+ client->refs++; -+ return client; -+} -+ -+RedClient *red_client_unref(RedClient *client) -+{ -+ if (!--client->refs) { -+ spice_debug("release client=%p", client); -+ pthread_mutex_destroy(&client->lock); -+ free(client); -+ return NULL; -+ } -+ return client; -+} -+ - /* client mutex should be locked before this call */ - static void red_channel_client_set_migration_seamless(RedChannelClient *rcc) - { -@@ -2012,9 +2031,7 @@ void red_client_destroy(RedClient *client) - spice_assert(rcc->send_data.size == 0); - red_channel_client_destroy(rcc); - } -- -- pthread_mutex_destroy(&client->lock); -- free(client); -+ red_client_unref(client); - } - - /* client->lock should be locked */ -diff --git a/server/red_channel.h b/server/red_channel.h -index ba299b6..0dd73ea 100644 ---- a/server/red_channel.h -+++ b/server/red_channel.h -@@ -561,10 +561,25 @@ struct RedClient { - is called */ - int seamless_migrate; - int num_migrated_channels; /* for seamless - number of channels that wait for migrate data*/ -+ int refs; - }; - - RedClient *red_client_new(int migrated); - -+/* -+ * disconnects all the client's channels (should be called from the client's thread) -+ */ -+void red_client_destroy(RedClient *client); -+ -+RedClient *red_client_ref(RedClient *client); -+ -+/* -+ * releases the client resources when refs == 0. -+ * We assume the red_client_derstroy was called before -+ * we reached refs==0 -+ */ -+RedClient *red_client_unref(RedClient *client); -+ - MainChannelClient *red_client_get_main(RedClient *client); - // main should be set once before all the other channels are created - void red_client_set_main(RedClient *client, MainChannelClient *mcc); -@@ -580,7 +595,5 @@ void red_client_semi_seamless_migrate_complete(RedClient *client); /* dst side * - int red_client_during_migrate_at_target(RedClient *client); - - void red_client_migrate(RedClient *client); --// disconnects all the client's channels (should be called from the client's thread) --void red_client_destroy(RedClient *client); - - #endif --- -1.8.3.1 - diff --git a/0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch b/0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch deleted file mode 100644 index fab6d5c..0000000 --- a/0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch +++ /dev/null @@ -1,51 +0,0 @@ -From 06ba03b7b32a2f0c7f78c82d8f399242526a0b45 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Fri, 26 Jul 2013 13:49:24 -0400 -Subject: [PATCH 3/8] main_dispatcher: add ref count protection to RedClient - instances - ---- - server/main_dispatcher.c | 6 ++++-- - 1 file changed, 4 insertions(+), 2 deletions(-) - -diff --git a/server/main_dispatcher.c b/server/main_dispatcher.c -index e7a451a..bf160dd 100644 ---- a/server/main_dispatcher.c -+++ b/server/main_dispatcher.c -@@ -97,6 +97,7 @@ static void main_dispatcher_handle_migrate_complete(void *opaque, - MainDispatcherMigrateSeamlessDstCompleteMessage *mig_complete = payload; - - reds_on_client_seamless_migrate_complete(mig_complete->client); -+ red_client_unref(mig_complete->client); - } - - static void main_dispatcher_handle_mm_time_latency(void *opaque, -@@ -104,6 +105,7 @@ static void main_dispatcher_handle_mm_time_latency(void *opaque, - { - MainDispatcherMmTimeLatencyMessage *msg = payload; - reds_set_client_mm_time_latency(msg->client, msg->latency); -+ red_client_unref(msg->client); - } - - void main_dispatcher_seamless_migrate_dst_complete(RedClient *client) -@@ -115,7 +117,7 @@ void main_dispatcher_seamless_migrate_dst_complete(RedClient *client) - return; - } - -- msg.client = client; -+ msg.client = red_client_ref(client); - dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE, - &msg); - } -@@ -129,7 +131,7 @@ void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency) - return; - } - -- msg.client = client; -+ msg.client = red_client_ref(client); - msg.latency = latency; - dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_SET_MM_TIME_LATENCY, - &msg); --- -1.8.3.1 - diff --git a/0004-decouple-disconnection-of-the-main-channel-from-clie.patch b/0004-decouple-disconnection-of-the-main-channel-from-clie.patch deleted file mode 100644 index 85d70ec..0000000 --- a/0004-decouple-disconnection-of-the-main-channel-from-clie.patch +++ /dev/null @@ -1,238 +0,0 @@ -From 8490f83e1f51ac8dbcac3c68c97827e0acb9ec4e Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Thu, 25 Jul 2013 14:19:21 -0400 -Subject: [PATCH 4/8] decouple disconnection of the main channel from client - destruction - -Fixes rhbz#918169 - -Some channels make direct calls to reds/main_channel routines. If -these routines try to read/write to the socket, and they get socket -error, main_channel_client_on_disconnect is called, and triggers -red_client_destroy. In order to prevent accessing expired references -to RedClient, RedChannelClient, or other objects (inside the original call, after -red_client_destroy has been called) I made the call to -red_client_destroy asynchronous with respect to main_channel_client_on_disconnect. -I added MAIN_DISPATCHER_CLIENT_DISCONNECT to main_dispatcher. -main_channel_client_on_disconnect pushes this msg to the dispatcher, -instead of calling directly to reds_client_disconnect. - -The patch uses RedClient ref-count in order to handle a case where -reds_client_disconnect is called directly (e.g., when a new client connects while -another one is connected), while there is already CLIENT_DISCONNECT msg -pending in the main_dispatcher. - -Examples: -(1) snd_worker.c - - snd_disconnect_channel() - channel->cleanup() //snd_playback_cleanup - reds_enable_mm_timer() - . - . - main_channel_push_multi_media_time()...socket_error - . - . - red_client_destory() - . - . - snd_disconnect_channel() - channel->cleanup() - celt051_encoder_destroy() - celt051_encoder_destory() // double release - -Note that this bug could have been solved by changing the order of -calls: e.g., channel->stream = NULL before calling cleanup, and -some other changes + reference counting. However, I found other -places in the code with similar problems, and I looked for a general -solution, at least till we redesign red_channel to handle reference -counting more consistently. - -(2) inputs_channel.c - - inputs_connect() - main_channel_client_push_notify()...socket_error - . - . - red_client_destory() - . - . - red_channel_client_create() // refers to client which is already destroyed - -(3) reds.c - - reds_handle_main_link() - main_channel_push_init() ...socket error - . - . - red_client_destory() - . - . - main_channel_client_start_net_test(mcc) // refers to mcc which is already destroyed - - This can explain the assert in rhbz#964136, comment #1 (but not the hang that occurred before). ---- - server/main_channel.c | 9 +++++---- - server/main_dispatcher.c | 37 +++++++++++++++++++++++++++++++++++++ - server/main_dispatcher.h | 7 +++++++ - server/reds.c | 1 + - server/reds.h | 2 ++ - 5 files changed, 52 insertions(+), 4 deletions(-) - -diff --git a/server/main_channel.c b/server/main_channel.c -index 233e633..fe032a6 100644 ---- a/server/main_channel.c -+++ b/server/main_channel.c -@@ -46,6 +46,7 @@ - #include "red_common.h" - #include "reds.h" - #include "migration_protocol.h" -+#include "main_dispatcher.h" - - #define ZERO_BUF_SIZE 4096 - -@@ -175,13 +176,13 @@ int main_channel_is_connected(MainChannel *main_chan) - return red_channel_is_connected(&main_chan->base); - } - --// when disconnection occurs, let reds shutdown all channels. This will trigger the --// real disconnection of main channel -+/* -+ * When the main channel is disconnected, disconnect the entire client. -+ */ - static void main_channel_client_on_disconnect(RedChannelClient *rcc) - { - spice_printerr("rcc=%p", rcc); -- reds_client_disconnect(rcc->client); --// red_channel_client_disconnect(rcc); -+ main_dispatcher_client_disconnect(rcc->client); - } - - RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t connection_id) -diff --git a/server/main_dispatcher.c b/server/main_dispatcher.c -index bf160dd..dbe1037 100644 ---- a/server/main_dispatcher.c -+++ b/server/main_dispatcher.c -@@ -41,6 +41,7 @@ enum { - MAIN_DISPATCHER_CHANNEL_EVENT = 0, - MAIN_DISPATCHER_MIGRATE_SEAMLESS_DST_COMPLETE, - MAIN_DISPATCHER_SET_MM_TIME_LATENCY, -+ MAIN_DISPATCHER_CLIENT_DISCONNECT, - - MAIN_DISPATCHER_NUM_MESSAGES - }; -@@ -59,6 +60,10 @@ typedef struct MainDispatcherMmTimeLatencyMessage { - uint32_t latency; - } MainDispatcherMmTimeLatencyMessage; - -+typedef struct MainDispatcherClientDisconnectMessage { -+ RedClient *client; -+} MainDispatcherClientDisconnectMessage; -+ - /* channel_event - calls core->channel_event, must be done in main thread */ - static void main_dispatcher_self_handle_channel_event( - int event, -@@ -108,6 +113,16 @@ static void main_dispatcher_handle_mm_time_latency(void *opaque, - red_client_unref(msg->client); - } - -+static void main_dispatcher_handle_client_disconnect(void *opaque, -+ void *payload) -+{ -+ MainDispatcherClientDisconnectMessage *msg = payload; -+ -+ spice_debug("client=%p", msg->client); -+ reds_client_disconnect(msg->client); -+ red_client_unref(msg->client); -+} -+ - void main_dispatcher_seamless_migrate_dst_complete(RedClient *client) - { - MainDispatcherMigrateSeamlessDstCompleteMessage msg; -@@ -137,6 +152,20 @@ void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency) - &msg); - } - -+void main_dispatcher_client_disconnect(RedClient *client) -+{ -+ MainDispatcherClientDisconnectMessage msg; -+ -+ if (!client->disconnecting) { -+ spice_debug("client %p", client); -+ msg.client = red_client_ref(client); -+ dispatcher_send_message(&main_dispatcher.base, MAIN_DISPATCHER_CLIENT_DISCONNECT, -+ &msg); -+ } else { -+ spice_debug("client %p already during disconnection", client); -+ } -+} -+ - static void dispatcher_handle_read(int fd, int event, void *opaque) - { - Dispatcher *dispatcher = opaque; -@@ -144,6 +173,11 @@ static void dispatcher_handle_read(int fd, int event, void *opaque) - dispatcher_handle_recv_read(dispatcher); - } - -+/* -+ * FIXME: -+ * Reds routines shouldn't be exposed. Instead reds.c should register the callbacks, -+ * and the corresponding operations should be made only via main_dispatcher. -+ */ - void main_dispatcher_init(SpiceCoreInterface *core) - { - memset(&main_dispatcher, 0, sizeof(main_dispatcher)); -@@ -160,4 +194,7 @@ void main_dispatcher_init(SpiceCoreInterface *core) - dispatcher_register_handler(&main_dispatcher.base, MAIN_DISPATCHER_SET_MM_TIME_LATENCY, - main_dispatcher_handle_mm_time_latency, - sizeof(MainDispatcherMmTimeLatencyMessage), 0 /* no ack */); -+ dispatcher_register_handler(&main_dispatcher.base, MAIN_DISPATCHER_CLIENT_DISCONNECT, -+ main_dispatcher_handle_client_disconnect, -+ sizeof(MainDispatcherClientDisconnectMessage), 0 /* no ack */); - } -diff --git a/server/main_dispatcher.h b/server/main_dispatcher.h -index 0c79ca8..522c7f9 100644 ---- a/server/main_dispatcher.h -+++ b/server/main_dispatcher.h -@@ -7,6 +7,13 @@ - void main_dispatcher_channel_event(int event, SpiceChannelEventInfo *info); - void main_dispatcher_seamless_migrate_dst_complete(RedClient *client); - void main_dispatcher_set_mm_time_latency(RedClient *client, uint32_t latency); -+/* -+ * Disconnecting the client is always executed asynchronously, -+ * in order to protect from expired references in the routines -+ * that triggered the client destruction. -+ */ -+void main_dispatcher_client_disconnect(RedClient *client); -+ - void main_dispatcher_init(SpiceCoreInterface *core); - - #endif //MAIN_DISPATCHER_H -diff --git a/server/reds.c b/server/reds.c -index 30d0652..c66ddc4 100644 ---- a/server/reds.c -+++ b/server/reds.c -@@ -537,6 +537,7 @@ void reds_client_disconnect(RedClient *client) - } - - if (!client || client->disconnecting) { -+ spice_debug("client %p already during disconnection", client); - return; - } - -diff --git a/server/reds.h b/server/reds.h -index c5c557d..1c5ae84 100644 ---- a/server/reds.h -+++ b/server/reds.h -@@ -136,6 +136,8 @@ void reds_handle_agent_mouse_event(const VDAgentMouseState *mouse_state); // use - extern struct SpiceCoreInterface *core; - - // Temporary measures to make splitting reds.c to inputs_channel.c easier -+ -+/* should be called only from main_dispatcher */ - void reds_client_disconnect(RedClient *client); - - // Temporary (?) for splitting main channel --- -1.8.3.1 - diff --git a/0005-reds-s-red_client_disconnect-red_channel_client_shut.patch b/0005-reds-s-red_client_disconnect-red_channel_client_shut.patch deleted file mode 100644 index 035f462..0000000 --- a/0005-reds-s-red_client_disconnect-red_channel_client_shut.patch +++ /dev/null @@ -1,56 +0,0 @@ -From 46c2ce8f1af0170a2c6a335cc743a3ddff2d96d0 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Thu, 25 Jul 2013 14:25:24 -0400 -Subject: [PATCH 5/8] reds: s/red_client_disconnect/red_channel_client_shutdown - inside callbacks - -When we want to disconnect the main channel from a callback, it is -safer to use red_channel_client_shutdown, instead of directly -destroying the client. It is also more consistent with how other -channels treat errors. -red_channel_client_shutdown will trigger socket error in the main channel. -Then, main_channel_client_on_disconnect will be called, -and eventually, main_dispatcher_client_disconnect. - -I didn't replace calls to reds_disconnect/reds_client_disconnect in -places where those calls were safe && that might need immediate client -disconnection. ---- - server/reds.c | 7 ++++--- - 1 file changed, 4 insertions(+), 3 deletions(-) - -diff --git a/server/reds.c b/server/reds.c -index c66ddc4..ae87c90 100644 ---- a/server/reds.c -+++ b/server/reds.c -@@ -879,7 +879,8 @@ static void vdi_port_on_free_self_token(void *opaque) - - static void vdi_port_remove_client(RedClient *client, void *opaque) - { -- reds_client_disconnect(client); -+ red_channel_client_shutdown(main_channel_client_get_base( -+ red_client_get_main(client))); - } - - /****************************************************************************/ -@@ -1009,7 +1010,7 @@ void reds_on_main_agent_start(MainChannelClient *mcc, uint32_t num_tokens) - - if (!client_added) { - spice_warning("failed to add client to agent"); -- reds_client_disconnect(rcc->client); -+ red_channel_client_shutdown(rcc); - return; - } - } else { -@@ -1126,7 +1127,7 @@ void reds_on_main_agent_data(MainChannelClient *mcc, void *message, size_t size) - reds_on_main_agent_monitors_config(mcc, message, size); - return; - case AGENT_MSG_FILTER_PROTO_ERROR: -- reds_disconnect(); -+ red_channel_client_shutdown(main_channel_client_get_base(mcc)); - return; - } - --- -1.8.3.1 - diff --git a/0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch b/0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch deleted file mode 100644 index fc1ee51..0000000 --- a/0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch +++ /dev/null @@ -1,42 +0,0 @@ -From 134b7f310de5120b233670d18641d32204f31318 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Thu, 25 Jul 2013 14:49:33 -0400 -Subject: [PATCH 6/8] snd_worker: fix memory leak of PlaybackChannel - -When the sequence of calls bellow occurs, the PlaybackChannel -is not released (snd_channel_put is not called for the -samples that refer to the channel). - - spice_server_playback_get_buffer - snd_channel_disconnect - spice_server_playback_put_samples ---- - server/snd_worker.c | 9 ++++----- - 1 file changed, 4 insertions(+), 5 deletions(-) - -diff --git a/server/snd_worker.c b/server/snd_worker.c -index d6ec47a..849f002 100644 ---- a/server/snd_worker.c -+++ b/server/snd_worker.c -@@ -1097,14 +1097,13 @@ SPICE_GNUC_VISIBLE void spice_server_playback_put_samples(SpicePlaybackInstance - PlaybackChannel *playback_channel; - AudioFrame *frame; - -- if (!sin->st->worker.connection) { -- return; -- } -- - frame = SPICE_CONTAINEROF(samples, AudioFrame, samples); - playback_channel = frame->channel; -- if (!snd_channel_put(&playback_channel->base) || !playback_channel->base.worker->connection) { -+ spice_assert(playback_channel); -+ if (!snd_channel_put(&playback_channel->base) || -+ sin->st->worker.connection != &playback_channel->base) { - /* lost last reference, channel has been destroyed previously */ -+ spice_info("audio samples belong to a disconnected channel"); - return; - } - spice_assert(playback_channel->base.active); --- -1.8.3.1 - diff --git a/0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch b/0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch deleted file mode 100644 index fd461be..0000000 --- a/0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch +++ /dev/null @@ -1,62 +0,0 @@ -From 02f44c137df99ed2e89699e49b64c13673b0cd06 Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Thu, 25 Jul 2013 15:07:43 -0400 -Subject: [PATCH 7/8] snd_worker/snd_disconnect_channel: don't call - snd_channel_put if the channel has already been disconnected - -The snd channels has one reference as long as their socket is active. -The playback channel has an additional reference for each frame that is -currently filled by the sound device. -Once the channel is disconnected (the socket has been freed and the -first reference is released) snd_disconnect_channel shouldn't release -a reference again. ---- - server/snd_worker.c | 20 ++++++++++---------- - 1 file changed, 10 insertions(+), 10 deletions(-) - -diff --git a/server/snd_worker.c b/server/snd_worker.c -index 849f002..3827416 100644 ---- a/server/snd_worker.c -+++ b/server/snd_worker.c -@@ -212,21 +212,20 @@ static void snd_disconnect_channel(SndChannel *channel) - { - SndWorker *worker; - -- if (!channel) { -+ if (!channel || !channel->stream) { - spice_debug("not connected"); - return; - } - spice_debug("%p", channel); - worker = channel->worker; -- if (channel->stream) { -- channel->cleanup(channel); -- red_channel_client_disconnect(worker->connection->channel_client); -- core->watch_remove(channel->stream->watch); -- channel->stream->watch = NULL; -- reds_stream_free(channel->stream); -- channel->stream = NULL; -- spice_marshaller_destroy(channel->send_data.marshaller); -- } -+ channel->cleanup(channel); -+ red_channel_client_disconnect(worker->connection->channel_client); -+ worker->connection->channel_client = NULL; -+ core->watch_remove(channel->stream->watch); -+ channel->stream->watch = NULL; -+ reds_stream_free(channel->stream); -+ channel->stream = NULL; -+ spice_marshaller_destroy(channel->send_data.marshaller); - snd_channel_put(channel); - worker->connection = NULL; - } -@@ -897,6 +896,7 @@ static SndChannel *__new_channel(SndWorker *worker, int size, uint32_t channel_i - int tos; - MainChannelClient *mcc = red_client_get_main(client); - -+ spice_assert(stream); - if ((flags = fcntl(stream->socket, F_GETFL)) == -1) { - spice_printerr("accept failed, %s", strerror(errno)); - goto error1; --- -1.8.3.1 - diff --git a/0008-log-improve-debug-information-related-to-client-disc.patch b/0008-log-improve-debug-information-related-to-client-disc.patch deleted file mode 100644 index 77a04ba..0000000 --- a/0008-log-improve-debug-information-related-to-client-disc.patch +++ /dev/null @@ -1,99 +0,0 @@ -From c2e46b926e0ee4226f0f93942e7fa2c5b409f73d Mon Sep 17 00:00:00 2001 -From: Yonit Halperin -Date: Fri, 26 Jul 2013 12:15:00 -0400 -Subject: [PATCH 8/8] log: improve debug information related to client - disconnection - ---- - server/red_channel.c | 9 ++++++--- - server/snd_worker.c | 7 ++++--- - 2 files changed, 10 insertions(+), 6 deletions(-) - -diff --git a/server/red_channel.c b/server/red_channel.c -index 9168b8a..d565634 100644 ---- a/server/red_channel.c -+++ b/server/red_channel.c -@@ -1112,6 +1112,7 @@ static void red_channel_client_ref(RedChannelClient *rcc) - static void red_channel_client_unref(RedChannelClient *rcc) - { - if (!--rcc->refs) { -+ spice_debug("destroy rcc=%p", rcc); - if (rcc->send_data.main.marshaller) { - spice_marshaller_destroy(rcc->send_data.main.marshaller); - } -@@ -1708,6 +1709,8 @@ static void red_channel_client_disconnect_dummy(RedChannelClient *rcc) - { - spice_assert(rcc->dummy); - if (ring_item_is_linked(&rcc->channel_link)) { -+ spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, rcc->channel, -+ rcc->channel->type, rcc->channel->id); - red_channel_remove_client(rcc); - } - rcc->dummy_connected = FALSE; -@@ -1715,8 +1718,6 @@ static void red_channel_client_disconnect_dummy(RedChannelClient *rcc) - - void red_channel_client_disconnect(RedChannelClient *rcc) - { -- spice_printerr("%p (channel %p type %d id %d)", rcc, rcc->channel, -- rcc->channel->type, rcc->channel->id); - if (rcc->dummy) { - red_channel_client_disconnect_dummy(rcc); - return; -@@ -1724,6 +1725,8 @@ void red_channel_client_disconnect(RedChannelClient *rcc) - if (!red_channel_client_is_connected(rcc)) { - return; - } -+ spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, rcc->channel, -+ rcc->channel->type, rcc->channel->id); - red_channel_client_pipe_clear(rcc); - if (rcc->stream->watch) { - rcc->channel->core->watch_remove(rcc->stream->watch); -@@ -2007,7 +2010,7 @@ void red_client_destroy(RedClient *client) - RingItem *link, *next; - RedChannelClient *rcc; - -- spice_printerr("destroy client with #channels %d", client->channels_num); -+ spice_printerr("destroy client %p with #channels=%d", client, client->channels_num); - if (!pthread_equal(pthread_self(), client->thread_id)) { - spice_warning("client->thread_id (0x%lx) != pthread_self (0x%lx)." - "If one of the threads is != io-thread && != vcpu-thread," -diff --git a/server/snd_worker.c b/server/snd_worker.c -index 3827416..ebddfcd 100644 ---- a/server/snd_worker.c -+++ b/server/snd_worker.c -@@ -201,8 +201,8 @@ static SndChannel *snd_channel_get(SndChannel *channel) - static SndChannel *snd_channel_put(SndChannel *channel) - { - if (!--channel->refs) { -+ spice_printerr("SndChannel=%p freed", channel); - free(channel); -- spice_printerr("sound channel freed"); - return NULL; - } - return channel; -@@ -216,7 +216,8 @@ static void snd_disconnect_channel(SndChannel *channel) - spice_debug("not connected"); - return; - } -- spice_debug("%p", channel); -+ spice_debug("SndChannel=%p rcc=%p type=%d", -+ channel, channel->channel_client, channel->channel_client->channel->type); - worker = channel->worker; - channel->cleanup(channel); - red_channel_client_disconnect(worker->connection->channel_client); -@@ -976,11 +977,11 @@ static void snd_disconnect_channel_client(RedChannelClient *rcc) - { - SndWorker *worker; - -- spice_debug(NULL); - spice_assert(rcc->channel); - spice_assert(rcc->channel->data); - worker = (SndWorker *)rcc->channel->data; - -+ spice_debug("channel-type=%d", rcc->channel->type); - if (worker->connection) { - spice_assert(worker->connection->channel_client == rcc); - snd_disconnect_channel(worker->connection); --- -1.8.3.1 - diff --git a/0009-red_worker-decrease-the-timeout-when-flushing-comman.patch b/0009-red_worker-decrease-the-timeout-when-flushing-comman.patch deleted file mode 100644 index a37b32b..0000000 --- a/0009-red_worker-decrease-the-timeout-when-flushing-comman.patch +++ /dev/null @@ -1,66 +0,0 @@ -From 6ced0f698507de02a67cfcd25b7ab07e8c423fad Mon Sep 17 00:00:00 2001 -Message-Id: <6ced0f698507de02a67cfcd25b7ab07e8c423fad.1377595745.git.uril@redhat.com> -From: Yonit Halperin -Date: Mon, 5 Aug 2013 12:10:15 -0400 -Subject: [PATCH] red_worker: decrease the timeout when flushing commands and waiting for the client. - -150 seconds is way too long period for holding the guest driver and -waiting for a response for the client. This timeout was 15 seconds, but -when off-screen surfaces ware introduced it was arbitrarily multiplied by -10. -Other existing related bugs emphasize why it is important to decrease -the timeout: -(1) 994211 - the qxl driver waits for an async-io reponse for 60 seconds - and after that, it switches to sync-io mode. Not only that the - driver might use invalid data (since it didn't wait for the query to - complete), falling back to sync-io mode introduces other errors. -(2) 994175 - spice server sometimes doesn't recognize that the client - has disconnected. -(3) There might be cache inconsistency between the client and the server, -and then the display channel waits indefinitely for a cache item (e.g., bug -977998) - -This patch changes the timeout to 30 seconds. I tested it under wifi +emulating 2.5Mbps network, -together with playing video on the guest and changing resolutions in a loop. The timeout didn't expired -during my tests. - -This bug is related to rhbz#964136 (but from rhbz#964136 info it is still not -clear why the client wasn't responsive). ---- - server/red_worker.c | 6 +++--- - 1 files changed, 3 insertions(+), 3 deletions(-) - -diff --git a/server/red_worker.c b/server/red_worker.c -index b45208b..9896696 100644 ---- a/server/red_worker.c -+++ b/server/red_worker.c -@@ -103,7 +103,7 @@ - #define CHANNEL_PUSH_TIMEOUT 30000000000ULL //nano - #define CHANNEL_PUSH_SLEEP_DURATION 10000 //micro - --#define DISPLAY_CLIENT_TIMEOUT 15000000000ULL //nano -+#define DISPLAY_CLIENT_TIMEOUT 30000000000ULL //nano - #define DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT 10000000000ULL //nano, 10 sec - #define DISPLAY_CLIENT_RETRY_INTERVAL 10000 //micro - -@@ -9716,7 +9716,7 @@ static inline void flush_display_commands(RedWorker *worker) - if (ring_is_empty) { - break; - } -- end_time = red_now() + DISPLAY_CLIENT_TIMEOUT * 10; -+ end_time = red_now() + DISPLAY_CLIENT_TIMEOUT; - int sleep_count = 0; - for (;;) { - red_channel_push(&worker->display_channel->common.base); -@@ -9760,7 +9760,7 @@ static inline void flush_cursor_commands(RedWorker *worker) - if (ring_is_empty) { - break; - } -- end_time = red_now() + DISPLAY_CLIENT_TIMEOUT * 10; -+ end_time = red_now() + DISPLAY_CLIENT_TIMEOUT; - int sleep_count = 0; - for (;;) { - red_channel_push(&worker->cursor_channel->common.base); --- -1.7.1 - diff --git a/sources b/sources index 0f55d1d..2e576dc 100644 --- a/sources +++ b/sources @@ -1 +1 @@ -325b1c42ce24e75de45a75876b73a8bd spice-0.12.4.tar.bz2 +1256286214fe402703c0a01bd3a85319 spice-0.12.5.tar.bz2 diff --git a/spice.spec b/spice.spec index cec90ee..36cb71b 100644 --- a/spice.spec +++ b/spice.spec @@ -1,21 +1,11 @@ Name: spice -Version: 0.12.4 -Release: 3%{?dist} +Version: 0.12.5 +Release: 1%{?dist} Summary: Implements the SPICE protocol Group: User Interface/Desktops License: LGPLv2+ URL: http://www.spice-space.org/ Source0: http://www.spice-space.org/download/releases/%{name}-%{version}.tar.bz2 -Patch1: 0001-red_channel-prevent-adding-and-pushing-pipe-items-af.patch -Patch2: 0002-red_channel-add-ref-count-to-RedClient.patch -Patch3: 0003-main_dispatcher-add-ref-count-protection-to-RedClien.patch -Patch4: 0004-decouple-disconnection-of-the-main-channel-from-clie.patch -Patch5: 0005-reds-s-red_client_disconnect-red_channel_client_shut.patch -Patch6: 0006-snd_worker-fix-memory-leak-of-PlaybackChannel.patch -Patch7: 0007-snd_worker-snd_disconnect_channel-don-t-call-snd_cha.patch -Patch8: 0008-log-improve-debug-information-related-to-client-disc.patch -Patch9: 0009-red_worker-decrease-the-timeout-when-flushing-comman.patch -Patch10: 0010-Fix-buffer-overflow-when-decrypting-client-SPICE-tic.patch # https://bugzilla.redhat.com/show_bug.cgi?id=613529 %if 0%{?rhel} @@ -110,6 +100,9 @@ mkdir -p %{buildroot}%{_libexecdir} %changelog +* Mon May 19 2014 Christophe Fergeau 0.12.5-1 +- Update to new 0.12.5 release + * Wed Oct 30 2013 Christophe Fergeau 0.12.4-3 - Add patch fixing CVE-2013-4282