[Spice-devel] [PATCH spice-server] fixup! dcc-send: remove useless pipe_item assignment pipe_item

[Uri Lublin]

On 7/4/19 12:33 PM, Frediano Ziglio wrote:
> Remove use-after-free introduced by a78a7d251042892182b158650291d19a85bbd6b1
> ---
>   server/dcc-send.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/server/dcc-send.c b/server/dcc-send.c
> index 565a79f33..4582e3545 100644
> --- a/server/dcc-send.c
> +++ b/server/dcc-send.c
> @@ -725,7 +725,6 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
>           RedPipeItem *pipe_item = l->data;
>           Drawable *drawable;
>           RedDrawablePipeItem *dpi;
> -        RedImageItem *image;
>   
>           if (pipe_item->type != RED_PIPE_ITEM_TYPE_DRAW)
>               continue;
> @@ -745,14 +744,16 @@ static void red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
>               continue;
>           }
>   
> -        image = dcc_add_surface_area_image(dcc, drawable->red_drawable->surface_id,
> -                                           &drawable->red_drawable->bbox, l, TRUE);
> +        dcc_add_surface_area_image(dcc, drawable->red_drawable->surface_id,
> +                                   &drawable->red_drawable->bbox, l, TRUE);
>           resent_surface_ids[num_resent] = drawable->red_drawable->surface_id;
>           resent_areas[num_resent] = drawable->red_drawable->bbox;
>           num_resent++;
>   
> -        spice_assert(image);
> +        GList *image_pos = l->next;

l may be the queue tail, and in that case l->next would be
NULL, wouldn't it ?

> +        spice_assert(image_pos);
>           red_channel_client_pipe_remove_and_release_pos(RED_CHANNEL_CLIENT(dcc), l);
> +        l = image_pos;
>       }
>   }
>   
> 

I solved it differently:

diff --git a/server/dcc-send.c b/server/dcc-send.c
index 84fa1be72..255e893f7 100644
--- a/server/dcc-send.c
+++ b/server/dcc-send.c
@@ -713,7 +713,7 @@ static void 
red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
      int resent_surface_ids[MAX_PIPE_SIZE];
      SpiceRect resent_areas[MAX_PIPE_SIZE]; // not pointers since 
drawables may be released
      int num_resent;
-    GList *l;
+    GList *l, *lprev;
      GQueue *pipe;

      resent_surface_ids[0] = first_surface_id;
@@ -723,12 +723,13 @@ static void 
red_pipe_replace_rendered_drawables_with_images(DisplayChannelClient
      pipe = red_channel_client_get_pipe(RED_CHANNEL_CLIENT(dcc));

      // going from the oldest to the newest
-    for (l = pipe->tail; l != NULL; l = l->prev) {
+    for (l = pipe->tail; l != NULL; l = lprev) {
          RedPipeItem *pipe_item = l->data;
          Drawable *drawable;
          RedDrawablePipeItem *dpi;
          RedImageItem *image;

+        lprev = l->prev;
          if (pipe_item->type != RED_PIPE_ITEM_TYPE_DRAW)
              continue;
          dpi = SPICE_UPCAST(RedDrawablePipeItem, pipe_item);




Uri.