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:
pgpA7yvrBFDIN.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel