Ack Series,
with the comment that the actual risky places that cause the crash, may
better use safe red_channel.c routines instead. But it can be done later.
On 07/08/2013 06:32 AM, Uri Lublin wrote:
If in RING_FOREACH (in == somewhere in the callee-tree) there is
a call to read or write, it is not safe, and RING_FOREACH_SAFE
should be used.
A call to such read/write that fails, may lead
to disconnecting from the client, which means
cleaning up things, and possibly removing from the ring
the specific node (channel/client) that is currently
being iterated by the FOREACH loop.
When the FOREACH loop tries to get the next item, spice-server
asserts and aborts in ring_next():
-- assertion `pos->next != NULL && pos->prev != NULL' failed
For example (first analysis done by Yonit):
In red_worker.c (WORKER_FOREACH_DCC):
red_pipes_add_drawable
red_pipe_add_drawable
red_handle_drawable_surfaces_client_synced
red_push_surface_image
red_channel_client_push
red_channel_client_send
red_peer_handle_outgoing
reds_stream_writev (if fails -- EPIPE)
handler->cb->on_error = red_channel_client_disconnect()
red_channel_remove_client()
ring_remove() -- of rcc from channel.clients ring.
In red_channel.c:
Commit 53488f0275d6c8a121af49f7ac817d09ce68090d fixes two cases of
such FOREACH loops.
Since the additional cost of a SAFE loop is insignificant, I replaced
all such FOREACH macros in red_worker.
In most pacthes, of red_worker, a FOREACH macro is modified to be safe.
In the last patch of red_worker, a generic SAFE_FOREACH is introduced.
If people prefer, I can reorder the patches such that generic macro is
introduced in the first patch, and the other patches to use it.
Last, the FOREACH macros can be used outside of red_worker.c.
Maybe in the future we can move it to an h-file and use it e.g. in red_worker.c
Uri Lublin (8):
red_worker: use only RCC_FOREACH_SAFE
red_worker: reuse DCC_FOREACH in WORKER_DCC_FOREACH
red_worker: make WORKER_FOREACH_DCC safe
red_worker: use only DRAWABLE_FOREACH_GLZ_SAFE
red_worker: make DRAWABLE_FOREACH_DPI safe
red_worker: make CCC_FOREACH safe
red_worker: use a generic SAFE_FOREACH macro
red_channel: replace RING_FOREACH with RING_FOREACH_SAFE in some
places
server/red_channel.c | 8 +-
server/red_worker.c | 190 +++++++++++++++++++++++---------------------------
2 files changed, 90 insertions(+), 108 deletions(-)
_______________________________________________
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