[Spice-devel] [server PATCH 0/8] use a SAFE version of RING_FOREACH (worker/channel)

[Uri Lublin]

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(-)