diff options
author | David Gibson <david@gibson.dropbear.id.au> | 2013-07-05 17:11:46 +1000 |
---|---|---|
committer | Hans de Goede <hdegoede@redhat.com> | 2013-07-05 14:59:58 +0200 |
commit | 53488f0275d6c8a121af49f7ac817d09ce68090d (patch) | |
tree | afb95cc4ced921e6bb64479b5be60ac050490253 | |
parent | b83c0fbf7f2eea9c66933bf51554778872f98174 (diff) |
Use RING_FOREACH_SAFE in red_channel.c functions which are missing it
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@redhat.com>
-rw-r--r-- | server/red_channel.c | 8 |
1 files changed, 4 insertions, 4 deletions
diff --git a/server/red_channel.c b/server/red_channel.c index c0b17819..8742008d 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) | |||
1572 | 1572 | ||
1573 | void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type) | 1573 | void red_channel_pipes_add_type(RedChannel *channel, int pipe_item_type) |
1574 | { | 1574 | { |
1575 | RingItem *link; | 1575 | RingItem *link, *next; |
1576 | 1576 | ||
1577 | RING_FOREACH(link, &channel->clients) { | 1577 | RING_FOREACH_SAFE(link, next, &channel->clients) { |
1578 | red_channel_client_pipe_add_type( | 1578 | red_channel_client_pipe_add_type( |
1579 | SPICE_CONTAINEROF(link, RedChannelClient, channel_link), | 1579 | SPICE_CONTAINEROF(link, RedChannelClient, channel_link), |
1580 | pipe_item_type); | 1580 | pipe_item_type); |
@@ -1593,9 +1593,9 @@ void red_channel_client_pipe_add_empty_msg(RedChannelClient *rcc, int msg_type) | |||
1593 | 1593 | ||
1594 | void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type) | 1594 | void red_channel_pipes_add_empty_msg(RedChannel *channel, int msg_type) |
1595 | { | 1595 | { |
1596 | RingItem *link; | 1596 | RingItem *link, *next; |
1597 | 1597 | ||
1598 | RING_FOREACH(link, &channel->clients) { | 1598 | RING_FOREACH_SAFE(link, next, &channel->clients) { |
1599 | red_channel_client_pipe_add_empty_msg( | 1599 | red_channel_client_pipe_add_empty_msg( |
1600 | SPICE_CONTAINEROF(link, RedChannelClient, channel_link), | 1600 | SPICE_CONTAINEROF(link, RedChannelClient, channel_link), |
1601 | msg_type); | 1601 | msg_type); |