[PATCH weston] Fix a crash when unlocking or unconfining a pointer

[Pekka Paalanen]

On Thu, 10 May 2018 00:53:38 -0700
Dima Ryazanov <dima at gmail.com> wrote:

> In GNOME (but not in Weston), if a window loses focus, the client first receives
> the focus event, then the unlock/unconfine event. This causes toytoolkit to
> dereference a NULL window when unlocking or unconfining the pointer.
> 
> To repro:
> - Run weston-confine
> - Click the window
> - Alt-Tab away from it
> 
> Result:
> 
> [1606837.869] wl_keyboard at 19.modifiers(63944, 524352, 0, 0, 0)
> [1606837.926] wl_keyboard at 19.leave(63945, wl_surface at 15)
> [1606837.945] wl_pointer at 18.leave(63946, wl_surface at 15)
> [1606837.956] wl_pointer at 18.frame()
> [1606837.961] zwp_confined_pointer_v1 at 26.unconfined()
> Segmentation fault (core dumped)
> 
> To fix this, get the input from the window instead of the other way around.
> 
> Signed-off-by: Dima Ryazanov <dima at gmail.com>
> ---
>  clients/window.c | 23 +++++++++++++----------
>  1 file changed, 13 insertions(+), 10 deletions(-)
> 

Hi Dima,

thank you for the patch, and sorry, I know you have some very old
patches still waiting for reviews.

> diff --git a/clients/window.c b/clients/window.c
> index bcf2b017..dee4455f 100644
> --- a/clients/window.c
> +++ b/clients/window.c
> @@ -286,6 +286,7 @@ struct window {
>  	confined_pointer_unconfined_handler_t pointer_unconfined_handler;
>  
>  	struct zwp_confined_pointer_v1 *confined_pointer;
> +	struct input *confined_input;
>  	struct widget *confined_widget;
>  	bool confined;
>  
> @@ -4788,8 +4789,8 @@ static void
>  locked_pointer_locked(void *data,
>  		      struct zwp_locked_pointer_v1 *locked_pointer)
>  {
> -	struct input *input = data;
> -	struct window *window = input->pointer_focus;
> +	struct window *window = data;
> +	struct input *input = window->locked_input;
>  
>  	window->pointer_locked = true;
>  
> @@ -4804,8 +4805,8 @@ static void
>  locked_pointer_unlocked(void *data,
>  			struct zwp_locked_pointer_v1 *locked_pointer)
>  {
> -	struct input *input = data;
> -	struct window *window = input->pointer_focus;
> +	struct window *window = data;
> +	struct input *input = window->locked_input;
>  
>  	window_unlock_pointer(window);
>  
> @@ -4860,7 +4861,7 @@ window_lock_pointer(struct window *window, struct input *input)
>  							ZWP_POINTER_CONSTRAINTS_V1_LIFETIME_ONESHOT);
>  	zwp_locked_pointer_v1_add_listener(locked_pointer,
>  					   &locked_pointer_listener,
> -					   input);
> +					   window);

Now that this object will have a pointer to window, how do we safeguard
against window_destroy() getting called before we dispatch a locked or
unlocked event? Wouldn't that be necessary?

>  
>  	window->locked_input = input;
>  	window->locked_pointer = locked_pointer;
> @@ -4902,8 +4903,8 @@ static void
>  confined_pointer_confined(void *data,
>  			  struct zwp_confined_pointer_v1 *confined_pointer)
>  {
> -	struct input *input = data;
> -	struct window *window = input->pointer_focus;
> +	struct window *window = data;
> +	struct input *input = window->confined_input;
>  
>  	window->confined = true;
>  
> @@ -4918,8 +4919,8 @@ static void
>  confined_pointer_unconfined(void *data,
>  			    struct zwp_confined_pointer_v1 *confined_pointer)
>  {
> -	struct input *input = data;
> -	struct window *window = input->pointer_focus;
> +	struct window *window = data;
> +	struct input *input = window->confined_input;
>  
>  	window_unconfine_pointer(window);
>  
> @@ -4984,8 +4985,9 @@ window_confine_pointer_to_rectangles(struct window *window,
>  
>  	zwp_confined_pointer_v1_add_listener(confined_pointer,
>  					     &confined_pointer_listener,
> -					     input);
> +					     window);

The same safeguard question here.

>  
> +	window->confined_input = input;

I was also wondering about what would clean up this field when 'input'
gets destroyed, but I suppose a proper event sequence will clean things
up before 'input' is destroyed.

>  	window->confined_pointer = confined_pointer;
>  	window->confined_widget = NULL;
>  
> @@ -5046,6 +5048,7 @@ window_unconfine_pointer(struct window *window)
>  	zwp_confined_pointer_v1_destroy(window->confined_pointer);
>  	window->confined_pointer = NULL;
>  	window->confined = false;
> +	window->confined_input = NULL;
>  }
>  
>  static void

However, I do think this is better than not, so pushed:
   da188835..e0dc5d47  master -> master

I also filed a spec bug:
https://bugs.freedesktop.org/show_bug.cgi?id=106704


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180529/796771db/attachment.sig>