[PATCH wayland v2] wayland-server: abort instead of posting events with wrong client objects

[Giulio Camuffo]

2016-12-08 20:36 GMT+01:00 Derek Foreman <derekf at osg.samsung.com>:
> Check that all the objects in an event belong to the same client as
> the resource posting it.  This prevents a compositor from accidentally
> mixing client objects and posting an event that causes a client to
> kill itself.
>
> It's intended that the compositor killing itself be easier to debug
> than the client killing itself for something that it's completely
> innocent of.

It may be easier, but as an user the compositor crashing is a highly
disruptive event, and i think we should strive to avoid it at all
times. By all means, log the problem, get a backtrace and print it,
maybe, but i don't agree with the abort(). Unless the problem really
is unrecoverable, but it seems this is not the case. If it was an
assert(0) it may be better, but i'm not sure i'd like that either.


Cheers,
Giulio

>
> Signed-off-by: Derek Foreman <derekf at osg.samsung.com>
> ---
>
> Changes since v1:
> uses get_next_arguments and arg_count_for_signature in the normal fashion
> abort instead of assert
>
>
> This does not address Pekka's request for a new_id test, as it's not
> immediately clear to me how to write it.
>
>  src/wayland-server.c | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/src/wayland-server.c b/src/wayland-server.c
> index 9d7d9c1..429dbef 100644
> --- a/src/wayland-server.c
> +++ b/src/wayland-server.c
> @@ -160,6 +160,33 @@ log_closure(struct wl_resource *resource,
>         }
>  }
>
> +static void
> +verify_objects(struct wl_resource *resource, uint32_t opcode,
> +             union wl_argument *args)
> +{
> +       struct wl_object *object = &resource->object;
> +       const char *signature = object->interface->events[opcode].signature;
> +       struct argument_details arg;
> +       struct wl_resource *res;
> +       int count, i;
> +
> +       count = arg_count_for_signature(signature);
> +       for (i = 0; i < count; i++) {
> +               signature = get_next_argument(signature, &arg);
> +               switch (arg.type) {
> +               case 'o':
> +                       res = (struct wl_resource *) (args[i].o);
> +                       if (res && res->client != resource->client) {
> +                               wl_log("unrecoverable error: The compositor "
> +                                      "tried to use an object from one "
> +                                      "client in an event for a different "
> +                                      "client.\n");
> +                               abort();
> +                       }
> +               }
> +       }
> +}
> +
>  WL_EXPORT void
>  wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode,
>                              union wl_argument *args)
> @@ -167,6 +194,7 @@ wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode,
>         struct wl_closure *closure;
>         struct wl_object *object = &resource->object;
>
> +       verify_objects(resource, opcode, args);
>         closure = wl_closure_marshal(object, opcode, args,
>                                      &object->interface->events[opcode]);
>
> @@ -206,6 +234,7 @@ wl_resource_queue_event_array(struct wl_resource *resource, uint32_t opcode,
>         struct wl_closure *closure;
>         struct wl_object *object = &resource->object;
>
> +       verify_objects(resource, opcode, args);
>         closure = wl_closure_marshal(object, opcode, args,
>                                      &object->interface->events[opcode]);
>
> --
> 2.10.2
>
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel