From 8480d13f6d9edd38483faa3538e2c96bf721c6b1 Mon Sep 17 00:00:00 2001 From: Alexandros Frantzis Date: Thu, 15 Feb 2018 13:07:09 +0200 Subject: [PATCH] libweston: Make weston_seat release safe 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 Reviewed-by: Pekka Paalanen Reviewed-by: Quentin Glidic --- compositor/text-backend.c | 8 +++-- libweston-desktop/seat.c | 16 ++++++--- libweston-desktop/wl-shell.c | 9 ++++- libweston-desktop/xdg-shell-v6.c | 24 +++++++++++++ libweston/data-device.c | 15 +++++--- libweston/input.c | 61 ++++++++++++++++++++++---------- 6 files changed, 102 insertions(+), 31 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/seat.c b/libweston-desktop/seat.c index 382b9e41..ae1c5e9f 100644 --- a/libweston-desktop/seat.c +++ b/libweston-desktop/seat.c @@ -242,6 +242,9 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat) struct wl_listener *listener; struct weston_desktop_seat *seat; + if (wseat == NULL) + return NULL; + listener = wl_signal_get(&wseat->destroy_signal, weston_desktop_seat_destroy); if (listener != NULL) @@ -270,7 +273,7 @@ weston_desktop_seat_from_seat(struct weston_seat *wseat) struct weston_desktop_surface * weston_desktop_seat_popup_grab_get_topmost_surface(struct weston_desktop_seat *seat) { - if (wl_list_empty(&seat->popup_grab.surfaces)) + if (seat == NULL || wl_list_empty(&seat->popup_grab.surfaces)) return NULL; struct wl_list *grab_link = seat->popup_grab.surfaces.next; @@ -282,11 +285,14 @@ bool weston_desktop_seat_popup_grab_start(struct weston_desktop_seat *seat, struct wl_client *client, uint32_t serial) { - assert(seat->popup_grab.client == NULL || seat->popup_grab.client == client); + assert(seat == NULL || seat->popup_grab.client == NULL || + seat->popup_grab.client == client); - struct weston_keyboard *keyboard = weston_seat_get_keyboard(seat->seat); - struct weston_pointer *pointer = weston_seat_get_pointer(seat->seat); - struct weston_touch *touch = weston_seat_get_touch(seat->seat); + struct weston_seat *wseat = seat != NULL ? seat->seat : NULL; + /* weston_seat_get_* functions can properly handle a NULL wseat */ + struct weston_keyboard *keyboard = weston_seat_get_keyboard(wseat); + struct weston_pointer *pointer = weston_seat_get_pointer(wseat); + struct weston_touch *touch = weston_seat_get_touch(wseat); if ((keyboard == NULL || keyboard->grab_serial != serial) && (pointer == NULL || pointer->grab_serial != serial) && diff --git a/libweston-desktop/wl-shell.c b/libweston-desktop/wl-shell.c index 66553f45..8467dfb8 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); } @@ -328,7 +334,8 @@ weston_desktop_wl_shell_surface_protocol_set_popup(struct wl_client *wl_client, struct weston_desktop_wl_shell_surface *surface = weston_desktop_surface_get_implementation_data(dsurface); - if (seat == NULL) { + /* Check that if we have a valid wseat we also got a valid desktop seat */ + if (wseat != NULL && seat == NULL) { wl_client_post_no_memory(wl_client); return; } diff --git a/libweston-desktop/xdg-shell-v6.c b/libweston-desktop/xdg-shell-v6.c index 4db3748b..f9902ff0 100644 --- a/libweston-desktop/xdg-shell-v6.c +++ b/libweston-desktop/xdg-shell-v6.c @@ -378,6 +378,9 @@ weston_desktop_xdg_toplevel_protocol_show_window_menu(struct wl_client *wl_clien return; } + if (seat == NULL) + return; + weston_desktop_api_show_window_menu(toplevel->base.desktop, dsurface, seat, x, y); } @@ -402,6 +405,9 @@ weston_desktop_xdg_toplevel_protocol_move(struct wl_client *wl_client, return; } + if (seat == NULL) + return; + weston_desktop_api_move(toplevel->base.desktop, dsurface, seat, serial); } @@ -428,6 +434,9 @@ weston_desktop_xdg_toplevel_protocol_resize(struct wl_client *wl_client, return; } + if (seat == NULL) + return; + weston_desktop_api_resize(toplevel->base.desktop, dsurface, seat, serial, surf_edges); } @@ -762,6 +771,12 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client, bool parent_is_toplevel = popup->parent->role == WESTON_DESKTOP_XDG_SURFACE_ROLE_TOPLEVEL; + /* Check that if we have a valid wseat we also got a valid desktop seat */ + if (wseat != NULL && seat == NULL) { + wl_client_post_no_memory(wl_client); + return; + } + if (popup->committed) { wl_resource_post_error(popup->resource, ZXDG_POPUP_V6_ERROR_INVALID_GRAB, @@ -769,6 +784,15 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client, return; } + /* If seat is NULL then get_topmost_surface will return NULL. In + * combination with setting parent_is_toplevel to TRUE here we will + * avoid posting an error, and we will instead gracefully fail the + * grab and dismiss the surface. + * FIXME: this is a hack because currently we cannot check the topmost + * parent with a destroyed weston_seat */ + if (seat == NULL) + parent_is_toplevel = true; + topmost = weston_desktop_seat_popup_grab_get_topmost_surface(seat); if ((topmost == NULL && !parent_is_toplevel) || (topmost != NULL && topmost != popup->parent->desktop_surface)) { 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)