Ack. Thanks! We have recently associated this problem with some opened
bugs we have. I believe Uri is working on a patch for a similar fix to
the red_pipes.* routines in red_worker.
On 07/05/2013 03:11 AM, David Gibson wrote:
Currently, both red_channel_pipes_add_type() and
red_channel_pipes_add_empty_msg() use plaing RING_FOREACH() which is not
safe versus removals from the ring within the loop body.
Although it's rare, such a removal can occur in both cases. In the case
of red_channel_pipes_add_type() we have:
red_channel_pipes_add_type()
-> red_channel_client_pipe_add_type()
-> red_channel_client_push()
And in the case of red_channel_client_pipes_add_empty_msg() we have:
red_channel_client_pipes_add_empty_msg()
-> red_channel_client_pipe_add_empty_msg()
-> red_channel_client_push()
But red_channel_client_push() can cause a removal from the clients ring if
a network error occurs:
red_channel_client_push()
-> red_channel_client_send()
-> red_peer_handle_outgoing()
-> handler->cb->on_error callback
= red_channel_client_default_peer_on_error()
-> red_channel_client_disconnect()
-> red_channel_remove_client()
-> ring_remove()
When this error path does occur, the assertion in RING_FOREACH()'s
ring_next() trips, and the process containing the spice server is aborted.
i.e. your whole VM dies, as a result of an unfortunately timed network
error on the spice channel.
Please apply.
Signed-off-by: David Gibson <dgibson@xxxxxxxxxx>
---
server/red_channel.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/server/red_channel.c b/server/red_channel.c
index c0b1781..8742008 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -1572,9 +1572,9 @@ void red_channel_client_pipe_add_type(RedChannelClient *rcc, int pipe_item_type)
void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type)
{
- RingItem *link;
+ RingItem *link, *next;
- RING_FOREACH(link, &channel->clients) {
+ RING_FOREACH_SAFE(link, next, &channel->clients) {
red_channel_client_pipe_add_type(
SPICE_CONTAINEROF(link, RedChannelClient, channel_link),
pipe_item_type);
@@ -1593,9 +1593,9 @@ void red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int msg_type)
void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type)
{
- RingItem *link;
+ RingItem *link, *next;
- RING_FOREACH(link, &channel->clients) {
+ RING_FOREACH_SAFE(link, next, &channel->clients) {
red_channel_client_pipe_add_empty_msg(
SPICE_CONTAINEROF(link, RedChannelClient, channel_link),
msg_type);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel