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

[Pekka Paalanen]

On Thu, 31 May 2018 00:16:14 -0700
Dima Ryazanov <dima at gmail.com> wrote:

> On Tue, May 29, 2018 at 3:30 AM Pekka Paalanen <ppaalanen at gmail.com> wrote:
> 
> > 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(-)

> > > @@ -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?
> >  
> 
> 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,
> > >                                            &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.
> >  
> 
> 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
-------------- 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/20180531/2bbb49f6/attachment.sig>