[PATCH v4 wayland] wayland-server: log an error for events with wrong client objects

[Yong Bakos]

Hi Derek,

> On Dec 12, 2016, at 12:39 PM, Derek Foreman <derekf at osg.samsung.com> 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 <derekf at osg.samsung.com>

Reviewed-by: Yong Bakos <ybakos at humanoriented.com>

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