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

[Carsten Haitzler (The Rasterman)]

On Fri, 9 Dec 2016 15:51:10 +0100 Giulio Camuffo <giuliocamuffo at gmail.com> said:

> 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.

i agree. aborts are VERY anti-social for a library to do. reporting an error
and making the function a "NOP" is by far the best thing. 

> 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
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    raster at rasterman.com