[PATCH weston v2 3/7] libweston: Make weston_seat release safe

Alexandros Frantzis alexandros.frantzis at collabora.com
Thu Feb 8 13:37:54 UTC 2018


Ensure the server can safely handle client requests for wl_seat resource
that have become inert due to weston_seat object release and subsequent
destruction.

The clean-up involves, among other things, unsetting the destroyed
weston_seat object from the user data of wl_seat resources, and handling
this NULL user data case where required.

The list of sites extracting and using weston_seat object from wl_seat
resources which were audited for this patch are:

Legend:
N/A = Not Applicable (not implemented by weston)
FIXED = Fixed in the commit
OK = Already works correctly

== keyboard_shortcuts_inhibit_unstable_v1 ==
[N/A] zwp_keyboard_shortcuts_inhibit_manager_v1.inhibit_shortcuts
== tablet_input_unstable_v{1,2} ==
[N/A] zwp_tablet_manager_v{1,2}.get_tablet_seat
== text_input_unstable_v1 ==
[FIXED] zwp_text_input_v1.activate
[FIXED] zwp_text_input_v1.deactivate
== wl_data_device ==
[FIXED] wl_data_device_manager.get_data_device
[OK] wl_data_device.start_drag
[FIXED] wl_data_device.set_selection
[OK] wl_data_device.release
== wl_shell ==
[FIXED] wl_shell_surface.move
[FIXED] wl_shell_surface.resize
[FIXED] wl_shell_surface.set_popup
== xdg_shell and xdg_shell_unstable_v6 ==
[FIXED] xdg_toplevel.show_window_menu
[FIXED] xdg_toplevel.move
[FIXED] xdg_toplevel.resize
[FIXED] xdg_popup.grab
== xdg_shell_unstable_v5 ==
[FIXED] xdg_shell.get_xdg_popup
[FIXED] xdg_surface.show_window_menu
[FIXED] xdg_surface.move
[FIXED] xdg_surface.resize

Signed-off-by: Alexandros Frantzis <alexandros.frantzis at collabora.com>
---
Changes in v2:
 - Properly create inert resources in seat_get_pointer/touch/keyboard.
 - Ensure all sites which have a wl_seat input resource can deal
   with inert resources.

 compositor/text-backend.c        |  8 ++++--
 libweston-desktop/wl-shell.c     | 12 +++++++-
 libweston-desktop/xdg-shell-v5.c | 16 ++++++++++-
 libweston-desktop/xdg-shell-v6.c | 18 +++++++++++-
 libweston/data-device.c          | 15 ++++++----
 libweston/input.c                | 61 ++++++++++++++++++++++++++++------------
 6 files changed, 102 insertions(+), 28 deletions(-)

diff --git a/compositor/text-backend.c b/compositor/text-backend.c
index e6ee249c..4d8c085b 100644
--- a/compositor/text-backend.c
+++ b/compositor/text-backend.c
@@ -193,10 +193,14 @@ text_input_activate(struct wl_client *client,
 {
 	struct text_input *text_input = wl_resource_get_user_data(resource);
 	struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
-	struct input_method *input_method = weston_seat->input_method;
+	struct input_method *input_method;
 	struct weston_compositor *ec = text_input->ec;
 	struct text_input *current;
 
+	if (!weston_seat)
+		return;
+
+	input_method = weston_seat->input_method;
 	if (input_method->input == text_input)
 		return;
 
@@ -237,7 +241,7 @@ text_input_deactivate(struct wl_client *client,
 {
 	struct weston_seat *weston_seat = wl_resource_get_user_data(seat);
 
-	if (weston_seat->input_method->input)
+	if (weston_seat && weston_seat->input_method->input)
 		deactivate_input_method(weston_seat->input_method);
 }
 
diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c
index 66553f45..3386d48b 100644
--- a/libweston-desktop/wl-shell.c
+++ b/libweston-desktop/wl-shell.c
@@ -220,6 +220,9 @@ weston_desktop_wl_shell_surface_protocol_move(struct wl_client *wl_client,
 	struct weston_desktop_wl_shell_surface *surface =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (seat == NULL)
+		return;
+
 	weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
 }
 
@@ -238,6 +241,9 @@ weston_desktop_wl_shell_surface_protocol_resize(struct wl_client *wl_client,
 	enum weston_desktop_surface_edge surf_edges =
 		(enum weston_desktop_surface_edge) edges;
 
+	if (seat == NULL)
+		return;
+
 	weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, surf_edges);
 }
 
@@ -321,13 +327,17 @@ weston_desktop_wl_shell_surface_protocol_set_popup(struct wl_client *wl_client,
 	struct weston_desktop_surface *dsurface =
 		wl_resource_get_user_data(resource);
 	struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
-	struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
+	struct weston_desktop_seat *seat;
 	struct weston_surface *parent =
 		wl_resource_get_user_data(parent_resource);
 	struct weston_desktop_surface *parent_surface;
 	struct weston_desktop_wl_shell_surface *surface =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (wseat == NULL)
+		return;
+
+	seat = weston_desktop_seat_from_seat(wseat);
 	if (seat == NULL) {
 		wl_client_post_no_memory(wl_client);
 		return;
diff --git a/libweston-desktop/xdg-shell-v5.c b/libweston-desktop/xdg-shell-v5.c
index ebe7940e..ac3de78a 100644
--- a/libweston-desktop/xdg-shell-v5.c
+++ b/libweston-desktop/xdg-shell-v5.c
@@ -421,6 +421,9 @@ weston_desktop_xdg_surface_protocol_show_window_menu(struct wl_client *wl_client
 	struct weston_desktop_xdg_surface *surface =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (seat == NULL)
+		return;
+
 	weston_desktop_xdg_surface_ensure_added(surface);
 	weston_desktop_api_show_window_menu(surface->desktop, dsurface, seat, x, y);
 }
@@ -438,6 +441,9 @@ weston_desktop_xdg_surface_protocol_move(struct wl_client *wl_client,
 	struct weston_desktop_xdg_surface *surface =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (seat == NULL)
+		return;
+
 	weston_desktop_xdg_surface_ensure_added(surface);
 	weston_desktop_api_move(surface->desktop, dsurface, seat, serial);
 }
@@ -458,6 +464,9 @@ weston_desktop_xdg_surface_protocol_resize(struct wl_client *wl_client,
 	enum weston_desktop_surface_edge surf_edges =
 		(enum weston_desktop_surface_edge) edges;
 
+	if (seat == NULL)
+		return;
+
 	weston_desktop_xdg_surface_ensure_added(surface);
 	weston_desktop_api_resize(surface->desktop, dsurface, seat, serial, surf_edges);
 }
@@ -766,11 +775,16 @@ weston_desktop_xdg_shell_protocol_get_xdg_popup(struct wl_client *wl_client,
 	struct weston_surface *wparent =
 		wl_resource_get_user_data(parent_resource);
 	struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
-	struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
+	struct weston_desktop_seat *seat;
 	struct weston_desktop_surface *parent, *topmost;
 	bool parent_is_popup, parent_is_xdg;
 	struct weston_desktop_xdg_popup *popup;
 
+	if (wseat == NULL)
+		return;
+
+	seat = weston_desktop_seat_from_seat(wseat);
+
 	if (weston_surface_set_role(wsurface, "xdg_popup", resource, XDG_SHELL_ERROR_ROLE) < 0)
 		return;
 
diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c
index 4db3748b..3bf15e7a 100644
--- a/libweston-desktop/xdg-shell-v6.c
+++ b/libweston-desktop/xdg-shell-v6.c
@@ -371,6 +371,9 @@ weston_desktop_xdg_toplevel_protocol_show_window_menu(struct wl_client *wl_clien
 	struct weston_desktop_xdg_toplevel *toplevel =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (seat == NULL)
+		return;
+
 	if (!toplevel->base.configured) {
 		wl_resource_post_error(toplevel->resource,
 				       ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
@@ -395,6 +398,9 @@ weston_desktop_xdg_toplevel_protocol_move(struct wl_client *wl_client,
 	struct weston_desktop_xdg_toplevel *toplevel =
 		weston_desktop_surface_get_implementation_data(dsurface);
 
+	if (seat == NULL)
+		return;
+
 	if (!toplevel->base.configured) {
 		wl_resource_post_error(toplevel->resource,
 				       ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
@@ -421,6 +427,9 @@ weston_desktop_xdg_toplevel_protocol_resize(struct wl_client *wl_client,
 	enum weston_desktop_surface_edge surf_edges =
 		(enum weston_desktop_surface_edge) edges;
 
+	if (seat == NULL)
+		return;
+
 	if (!toplevel->base.configured) {
 		wl_resource_post_error(toplevel->resource,
 				       ZXDG_SURFACE_V6_ERROR_NOT_CONSTRUCTED,
@@ -757,11 +766,18 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client,
 	struct weston_desktop_xdg_popup *popup =
 		weston_desktop_surface_get_implementation_data(dsurface);
 	struct weston_seat *wseat = wl_resource_get_user_data(seat_resource);
-	struct weston_desktop_seat *seat = weston_desktop_seat_from_seat(wseat);
+	struct weston_desktop_seat *seat;
 	struct weston_desktop_surface *topmost;
 	bool parent_is_toplevel =
 		popup->parent->role == WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL;
 
+	if (wseat == NULL) {
+		weston_desktop_surface_popup_dismiss(popup->base.desktop_surface);
+		return;
+	}
+
+	seat = weston_desktop_seat_from_seat(wseat);
+
 	if (popup->committed) {
 		wl_resource_post_error(popup->resource,
 				       ZXDG_POPUP_V6_ERROR_INVALID_GRAB,
diff --git a/libweston/data-device.c b/libweston/data-device.c
index b4bb4b37..e3dbee3e 100644
--- a/libweston/data-device.c
+++ b/libweston/data-device.c
@@ -1167,9 +1167,10 @@ data_device_set_selection(struct wl_client *client,
 			  struct wl_resource *resource,
 			  struct wl_resource *source_resource, uint32_t serial)
 {
+	struct weston_seat *seat = wl_resource_get_user_data(resource);
 	struct weston_data_source *source;
 
-	if (!source_resource)
+	if (!seat || !source_resource)
 		return;
 
 	source = wl_resource_get_user_data(source_resource);
@@ -1182,8 +1183,7 @@ data_device_set_selection(struct wl_client *client,
 	}
 
 	/* FIXME: Store serial and check against incoming serial here. */
-	weston_seat_set_selection(wl_resource_get_user_data(resource),
-				  source, serial);
+	weston_seat_set_selection(seat, source, serial);
 }
 static void
 data_device_release(struct wl_client *client, struct wl_resource *resource)
@@ -1296,8 +1296,13 @@ get_data_device(struct wl_client *client,
 		return;
 	}
 
-	wl_list_insert(&seat->drag_resource_list,
-		       wl_resource_get_link(resource));
+	if (seat) {
+		wl_list_insert(&seat->drag_resource_list,
+			       wl_resource_get_link(resource));
+	} else {
+		wl_list_init(wl_resource_get_link(resource));
+	}
+
 	wl_resource_set_implementation(resource, &data_device_interface,
 				       seat, unbind_data_device);
 }
diff --git a/libweston/input.c b/libweston/input.c
index 647268af..da002548 100644
--- a/libweston/input.c
+++ b/libweston/input.c
@@ -2420,13 +2420,10 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
 	 * This prevents a race between the compositor sending new
 	 * capabilities and the client trying to use the old ones.
 	 */
-	struct weston_pointer *pointer = seat->pointer_state;
+	struct weston_pointer *pointer = seat ? seat->pointer_state : NULL;
 	struct wl_resource *cr;
 	struct weston_pointer_client *pointer_client;
 
-	if (!pointer)
-		return;
-
         cr = wl_resource_create(client, &wl_pointer_interface,
 				wl_resource_get_version(resource), id);
 	if (cr == NULL) {
@@ -2434,6 +2431,15 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
 		return;
 	}
 
+	wl_list_init(wl_resource_get_link(cr));
+	wl_resource_set_implementation(cr, &pointer_interface, pointer,
+				       unbind_pointer_client_resource);
+
+	/* If we don't have a pointer_state, the resource is inert, so there
+	 * is nothing more to set up */
+	if (!pointer)
+		return;
+
 	pointer_client = weston_pointer_ensure_pointer_client(pointer, client);
 	if (!pointer_client) {
 		wl_client_post_no_memory(client);
@@ -2442,8 +2448,6 @@ seat_get_pointer(struct wl_client *client, struct wl_resource *resource,
 
 	wl_list_insert(&pointer_client->pointer_resources,
 		       wl_resource_get_link(cr));
-	wl_resource_set_implementation(cr, &pointer_interface, pointer,
-				       unbind_pointer_client_resource);
 
 	if (pointer->focus && pointer->focus->surface->resource &&
 	    wl_resource_get_client(pointer->focus->surface->resource) == client) {
@@ -2507,12 +2511,9 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource,
 	 * This prevents a race between the compositor sending new
 	 * capabilities and the client trying to use the old ones.
 	 */
-	struct weston_keyboard *keyboard = seat->keyboard_state;
+	struct weston_keyboard *keyboard = seat ? seat->keyboard_state : NULL;
 	struct wl_resource *cr;
 
-	if (!keyboard)
-		return;
-
         cr = wl_resource_create(client, &wl_keyboard_interface,
 				wl_resource_get_version(resource), id);
 	if (cr == NULL) {
@@ -2520,12 +2521,19 @@ seat_get_keyboard(struct wl_client *client, struct wl_resource *resource,
 		return;
 	}
 
+	wl_list_init(wl_resource_get_link(cr));
+	wl_resource_set_implementation(cr, &keyboard_interface,
+				       keyboard, unbind_resource);
+
+	/* If we don't have a keyboard_state, the resource is inert, so there
+	 * is nothing more to set up */
+	if (!keyboard)
+		return;
+
 	/* May be moved to focused list later by either
 	 * weston_keyboard_set_focus or directly if this client is already
 	 * focused */
 	wl_list_insert(&keyboard->resource_list, wl_resource_get_link(cr));
-	wl_resource_set_implementation(cr, &keyboard_interface,
-				       keyboard, unbind_resource);
 
 	if (wl_resource_get_version(cr) >= WL_KEYBOARD_REPEAT_INFO_SINCE_VERSION) {
 		wl_keyboard_send_repeat_info(cr,
@@ -2587,12 +2595,9 @@ seat_get_touch(struct wl_client *client, struct wl_resource *resource,
 	 * This prevents a race between the compositor sending new
 	 * capabilities and the client trying to use the old ones.
 	 */
-	struct weston_touch *touch = seat->touch_state;
+	struct weston_touch *touch = seat ? seat->touch_state : NULL;
 	struct wl_resource *cr;
 
-	if (!touch)
-		return;
-
         cr = wl_resource_create(client, &wl_touch_interface,
 				wl_resource_get_version(resource), id);
 	if (cr == NULL) {
@@ -2600,6 +2605,15 @@ seat_get_touch(struct wl_client *client, struct wl_resource *resource,
 		return;
 	}
 
+	wl_list_init(wl_resource_get_link(cr));
+	wl_resource_set_implementation(cr, &touch_interface,
+				       touch, unbind_resource);
+
+	/* If we don't have a touch_state, the resource is inert, so there
+	 * is nothing more to set up */
+	if (!touch)
+		return;
+
 	if (touch->focus &&
 	    wl_resource_get_client(touch->focus->surface->resource) == client) {
 		wl_list_insert(&touch->focus_resource_list,
@@ -2608,8 +2622,6 @@ seat_get_touch(struct wl_client *client, struct wl_resource *resource,
 		wl_list_insert(&touch->resource_list,
 			       wl_resource_get_link(cr));
 	}
-	wl_resource_set_implementation(cr, &touch_interface,
-				       touch, unbind_resource);
 }
 
 static void
@@ -3087,6 +3099,19 @@ weston_seat_init(struct weston_seat *seat, struct weston_compositor *ec,
 WL_EXPORT void
 weston_seat_release(struct weston_seat *seat)
 {
+	struct wl_resource *resource;
+
+	wl_resource_for_each(resource, &seat->base_resource_list) {
+		wl_resource_set_user_data(resource, NULL);
+	}
+
+	wl_resource_for_each(resource, &seat->drag_resource_list) {
+		wl_resource_set_user_data(resource, NULL);
+	}
+
+	wl_list_remove(&seat->base_resource_list);
+	wl_list_remove(&seat->drag_resource_list);
+
 	wl_list_remove(&seat->link);
 
 	if (seat->saved_kbd_focus)
-- 
2.14.1



More information about the wayland-devel mailing list