connection: Move object lookup out of wl_connection_demarshal()thread-safety

On the client side where we queue up multiple events before dispatching, we
need to look up the receiving proxy and argument proxies immediately before
calling the handler.  Between queueing up multiple events and eventually
invoking the handler, previous handlers may have destroyed some of the
proxies.

4 files changed, 67 insertions, 32 deletions

diff --git a/src/connection.c b/src/connection.c
index 54de4f1..881e8e8 100644
--- a/src/connection.c
+++ b/src/connection.c
@@ -699,12 +699,13 @@ wl_connection_demarshal(struct wl_connection *connection,
 			break;
 		case 'o':
 			closure->types[i] = &ffi_type_pointer;
-			object = (struct wl_object **) extra;
-			extra += sizeof *object;
-			closure->args[i] = object;
+			id = (uint32_t **) extra;
+			extra += sizeof *id;
+			closure->args[i] = id;
+			*id = p;
 
-			if (*p == 0 && !arg.nullable) {
-				printf("NULL new ID received on non-nullable "
+			if (*id == 0 && !arg.nullable) {
+				printf("NULL object received on non-nullable "
 				       "type, message %s(%s)\n", message->name,
 				       message->signature);
 				*object = NULL;
@@ -712,29 +713,6 @@ wl_connection_demarshal(struct wl_connection *connection,
 				goto err;
 			}
 
-			*object = wl_map_lookup(objects, *p);
-			if (*object == WL_ZOMBIE_OBJECT) {
-				/* references object we've already
-				 * destroyed client side */
-				*object = NULL;
-			} else if (*object == NULL && *p != 0) {
-				printf("unknown object (%u), message %s(%s)\n",
-				       *p, message->name, message->signature);
-				*object = NULL;
-				errno = EINVAL;
-				goto err;
-			}
-
-			if (*object != NULL && message->types[i-2] != NULL &&
-			    (*object)->interface != message->types[i-2]) {
-				printf("invalid object (%u), type (%s), "
-					"message %s(%s)\n",
-				       *p, (*object)->interface->name,
-				       message->name, message->signature);
-				errno = EINVAL;
-				goto err;
-			}
-
 			p++;
 			break;
 		case 'n':
@@ -821,6 +799,53 @@ wl_connection_demarshal(struct wl_connection *connection,
 	return NULL;
 }
 
+int
+wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects)
+{
+	struct wl_object **object;
+	const struct wl_message *message;
+	const char *signature;
+	struct argument_details arg;
+	int i, count;
+	uint32_t id;
+
+	message = closure->message;
+	signature = message->signature;
+	count = arg_count_for_signature(signature) + 2;
+	for (i = 2; i < count; i++) {
+		signature = get_next_argument(signature, &arg);
+		switch (arg.type) {
+		case 'o':
+			id = **(uint32_t **) closure->args[i];
+			object = closure->args[i];
+			*object = wl_map_lookup(objects, id);
+			if (*object == WL_ZOMBIE_OBJECT) {
+				/* references object we've already
+				 * destroyed client side */
+				*object = NULL;
+			} else if (*object == NULL && id != 0) {
+				printf("unknown object (%u), message %s(%s)\n",
+				       id, message->name, message->signature);
+				*object = NULL;
+				errno = EINVAL;
+				return -1;
+			}
+
+			if (*object != NULL && message->types[i-2] != NULL &&
+			    (*object)->interface != message->types[i-2]) {
+				printf("invalid object (%u), type (%s), "
+				       "message %s(%s)\n",
+				       id, (*object)->interface->name,
+				       message->name, message->signature);
+				errno = EINVAL;
+				return -1;
+			}
+		}
+	}
+
+	return 0;
+}
+
 void
 wl_closure_invoke(struct wl_closure *closure,
 		  struct wl_object *target, void (*func)(void), void *data)
diff --git a/src/wayland-client.c b/src/wayland-client.c
index 0b988fd..f7ed284 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -520,7 +520,7 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
 	struct wl_closure *closure;
 	struct wl_proxy *proxy;
 	uint32_t id;
-	int opcode;
+	int opcode, ret;
 
 	closure = container_of(queue->event_list.next,
 			       struct wl_closure, link);
@@ -528,14 +528,21 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
 	id = closure->buffer[0];
 	opcode = closure->buffer[1] & 0xffff;
 
+	/* Verify that the receiving object is still valid and look up
+	 * proxies for any arguments.  We have to do this just before
+	 * calling the handler, since preceeding events may have
+	 * destroyed either the proxy or the proxy args since the
+	 * event was queued. */
 	proxy = wl_map_lookup(&display->objects, id);
+	ret = wl_closure_lookup_objects(closure, &display->objects);
 
 	pthread_mutex_unlock(&display->mutex);
 
-	if (proxy != WL_ZOMBIE_OBJECT)
+	if (proxy != WL_ZOMBIE_OBJECT && ret == 0)
 		wl_closure_invoke(closure, &proxy->object,
 				  proxy->object.implementation[opcode],
 				  proxy->user_data);
+
 	wl_closure_destroy(closure);
 
 	pthread_mutex_lock(&display->mutex);
@@ -546,7 +553,6 @@ WL_EXPORT int
 wl_display_dispatch_queue(struct wl_display *display,
 			  struct wl_event_queue *queue)
 {
-	struct wl_closure *closure;
 	int len, size;
 
 	pthread_mutex_lock(&display->mutex);
diff --git a/src/wayland-private.h b/src/wayland-private.h
index 9c743e4..8adee9f 100644
--- a/src/wayland-private.h
+++ b/src/wayland-private.h
@@ -99,6 +99,9 @@ wl_connection_demarshal(struct wl_connection *connection,
 			struct wl_map *objects,
 			const struct wl_message *message);
 
+int
+wl_closure_lookup_objects(struct wl_closure *closure, struct wl_map *objects);
+
 void
 wl_closure_invoke(struct wl_closure *closure,
 		  struct wl_object *target, void (*func)(void), void *data);
diff --git a/src/wayland-server.c b/src/wayland-server.c
index 7573013..83c0778 100644
--- a/src/wayland-server.c
+++ b/src/wayland-server.c
@@ -278,7 +278,8 @@ wl_client_connection_data(int fd, uint32_t mask, void *data)
 		if (wl_debug)
 			wl_closure_print(closure, object, false);
 
-		if (closure == NULL && errno == EINVAL) {
+		if ((closure == NULL && errno == EINVAL) ||
+		    wl_closure_lookup_objects(closure, &client->objects) < 0) {
 			wl_resource_post_error(client->display_resource,
 					       WL_DISPLAY_ERROR_INVALID_METHOD,
 					       "invalid arguments for %s@%u.%s",