On Fri, Jul 05, 2013 at 05:11:46PM +1000, 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.

Looks good to me, thanks for tracking it down! I'll let other people more
familiar with spice give a final ACK

Christophe

Attachment: pgpBCwp6yTw6h.pgp
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel

Reply via email to