[PATCH] server: Fix crash when accessing client which is already freed

[Giulio Camuffo]

Hi,

could you write how to trigger the crash, also in the commit message
maybe? Besides that, i have a comment inline below.

2016-09-21 9:08 GMT+02:00 Hyunkook Khang <hyunkook.khang at lge.com>:
> While processing pending data, client could be destroyed in the middle of
> the process. (e.g. by invoking wl_display_flush_clients()).
> In this case, client will be freed, but we are still in the processing data
> of the client, so it could cause a crash.
>
> To avoid this, instead of destroying the client directly,
> just set the error here and destroy the client where it needs to be.
>
> Signed-off-by: Hyunkook Khang <hyunkook.khang at lge.com>
> ---
>  src/wayland-server.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 9d7d9c1..89d0bac 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -1103,10 +1103,16 @@ wl_display_terminate(struct wl_display *display)
>  WL_EXPORT void
>  wl_display_run(struct wl_display *display)
>  {
> +       struct wl_client *client, *next;
> +
>         display->run = 1;
>
>         while (display->run) {
>                 wl_display_flush_clients(display);
> +               wl_list_for_each_safe(client, next, &display->client_list, link) {
> +                       if (client->error)
> +                               wl_client_destroy(client);
> +               }

wl_display_flush_clients() may be called manually by the user too,
without using wl_display_run(). So now all the users would need to do
the loop to destroy the clients, or else they would leak. Instead, you
should do the loop at the end of wl_display_flush_clients().


Thanks,
Giulio

>                 wl_event_loop_dispatch(display->loop, -1);
>         }
>  }
> @@ -1124,7 +1130,7 @@ wl_display_flush_clients(struct wl_display *display)
>                                                   WL_EVENT_WRITABLE |
>                                                   WL_EVENT_READABLE);
>                 } else if (ret < 0) {
> -                       wl_client_destroy(client);
> +                       client->error = 1;
>                 }
>         }
>  }
> --
> 1.7.9.5
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel