subject:"\[PATCH weston\] Fix a crash when unlocking or unconfining a pointer"

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

2018-05-31 Thread Pekka Paalanen

On Thu, 31 May 2018 00:16:14 -0700
Dima Ryazanov  wrote:

> On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen  wrote:
> 
> > On Thu, 10 May 2018 00:53:38 -0700
> > Dima Ryazanov  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@19.modifiers(63944, 524352, 0, 0, 0)
> > > [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
> > > [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
> > > [1606837.956] wl_pointer@18.frame()
> > > [1606837.961] zwp_confined_pointer_v1@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 
> > > ---
> > >  clients/window.c | 23 +--
> > >  1 file changed, 13 insertions(+), 10 deletions(-)

> > > @@ -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,
> > >  _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?
> >  
> 
> Ah, interesting, I did not think about that. I'll see if I can make it
> happen. I'm guessing window_destroy should call window_unconfine_pointer and
> window_unlock_pointer.

> > > @@ -4984,8 +4985,9 @@ window_confine_pointer_to_rectangles(struct window  
> > *window,  
> > >
> > >   zwp_confined_pointer_v1_add_listener(confined_pointer,
> > >_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.
> >  
> 
> Interesting. I can't convince myself that cleanup will happen correctly.
> Now that I think about it, I wonder if I should've kept "input" in event
> handlers - but instead of accessing input->pointer_focus, I should've added
> new variables (confined_window, locked_window?).
> 
> Anyways, I'll try a few things. Still trying to figure out how all of this
> works.

Good luck! I appreciate the effort and I would be exactly in the same
place, not having much clue about how window.c works today. :-)


Thanks,
pq


pgpDgKg5MCMok.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

2018-05-31 Thread Dima Ryazanov

On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen  wrote:

> On Thu, 10 May 2018 00:53:38 -0700
> Dima Ryazanov  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@19.modifiers(63944, 524352, 0, 0, 0)
> > [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
> > [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
> > [1606837.956] wl_pointer@18.frame()
> > [1606837.961] zwp_confined_pointer_v1@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 
> > ---
> >  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.
>

Haha, no worries!


> > 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,
> >  _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?
>

Ah, interesting, I did not think about that. I'll see if I can make it
happen. I'm guessing window_destroy should call window_unconfine_pointer and
window_unlock_pointer.

>
> >   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,
> >_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.
>

Interesting. I can't convince myself that cleanup will happen correctly.
Now that I think about it, I wonder if I should've kept "input" in event
handlers - but instead of accessing input->pointer_focus, I should've added
new variables (confined_window, locked_window?).

Anyways, I'll try a few things. Still trying to figure out how

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

2018-05-29 Thread Pekka Paalanen

On Thu, 10 May 2018 00:53:38 -0700
Dima Ryazanov  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@19.modifiers(63944, 524352, 0, 0, 0)
> [1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
> [1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
> [1606837.956] wl_pointer@18.frame()
> [1606837.961] zwp_confined_pointer_v1@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 
> ---
>  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,
>  _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,
>_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


pgphkhEieXH_1.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org

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

2018-05-10 Thread Dima Ryazanov

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@19.modifiers(63944, 524352, 0, 0, 0)
[1606837.926] wl_keyboard@19.leave(63945, wl_surface@15)
[1606837.945] wl_pointer@18.leave(63946, wl_surface@15)
[1606837.956] wl_pointer@18.frame()
[1606837.961] zwp_confined_pointer_v1@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 
---
 clients/window.c | 23 +--
 1 file changed, 13 insertions(+), 10 deletions(-)

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,
   _pointer_listener,
-  input);
+  window);
 
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,
 _pointer_listener,
-input);
+window);
 
+   window->confined_input = input;
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
-- 
2.17.0

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel

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

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

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

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

4 matches

Site Navigation

Mail list logo

Footer information