On Mon, Dec 12, 2016 at 12:46:30PM -0800, Yong Bakos wrote: > Hi Derek, > > > On Dec 12, 2016, at 12:39 PM, Derek Foreman <[email protected]> wrote: > > > > 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 > > abort with a cryptic message. > > > > Instead the client will now be disconnected as it is when the compositor > > tries to send a null for a non-nullable object, and a log message > > will be printed by the compositor. > > > > Signed-off-by: Derek Foreman <[email protected]> > > Reviewed-by: Yong Bakos <[email protected]>
Looks like a good approach to flag the potential issue. And agreed, logging an error is probably friendlier than forcing users to terminate their server. Reviewed-by: Bryce Harrington <[email protected]> > yong > > > > --- > > src/wayland-server.c | 38 ++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > > > This started life as an assert, became an abort, and now it's a log > > and disconnect. Log and disconnect is already how we manage nullable > > violation on the compositor side, so I'm hoping it won't be too > > controversial. > > > > For EFL clients the disconnect is recoverable under some circumstances with > > our session recovery protocol, but the current client-abort()s behaviour > > is not. > > > > Changes from v1: > > uses get_next_arguemnts and arg_count_for_signature instead of a bespoke > > implementation. > > > > Changes from v2: > > Tests new_id objects as well > > logs and disconnects instead of assert()/abort() > > superficial changes to the log text > > > > Changes from v3: > > Actually printing the interface name and message name make this > > useful for debugging - instead of actually less informative > > than it was with a client side error. > > > > diff --git a/src/wayland-server.c b/src/wayland-server.c > > index 9d7d9c1..023d427 100644 > > --- a/src/wayland-server.c > > +++ b/src/wayland-server.c > > @@ -160,6 +160,36 @@ log_closure(struct wl_resource *resource, > > } > > } > > > > +static bool > > +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 'n': > > + case 'o': > > + res = (struct wl_resource *) (args[i].o); > > + if (res && res->client != resource->client) { > > + wl_log("compositor bug: The compositor " > > + "tried to use an object from one " > > + "client in a '%s.%s' for a different " > > + "client.\n", object->interface->name, > > + object->interface->events[opcode].name); > > + return false; > > + } > > + } > > + } > > + return true; > > +} > > + > > WL_EXPORT void > > wl_resource_post_event_array(struct wl_resource *resource, uint32_t opcode, > > union wl_argument *args) > > @@ -167,6 +197,10 @@ wl_resource_post_event_array(struct wl_resource > > *resource, uint32_t opcode, > > struct wl_closure *closure; > > struct wl_object *object = &resource->object; > > > > + if (!verify_objects(resource, opcode, args)) { > > + resource->client->error = 1; > > + return; > > + } > > closure = wl_closure_marshal(object, opcode, args, > > &object->interface->events[opcode]); > > > > @@ -206,6 +240,10 @@ wl_resource_queue_event_array(struct wl_resource > > *resource, uint32_t opcode, > > struct wl_closure *closure; > > struct wl_object *object = &resource->object; > > > > + if (!verify_objects(resource, opcode, args)) { > > + resource->client->error = 1; > > + return; > > + } > > closure = wl_closure_marshal(object, opcode, args, > > &object->interface->events[opcode]); > > > > -- > > 2.11.0 > > > > _______________________________________________ > > wayland-devel mailing list > > [email protected] > > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > > _______________________________________________ > wayland-devel mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
