[PATCH] touch-input: Don't dereference NULL pointer during full screen fade

[Pekka Paalanen]

On Fri, 29 Aug 2014 13:18:54 -0500
Derek Foreman <derekf at osg.samsung.com> wrote:

> I should mention I don't have a touch capable screen and am only doing
> partial testing with a modified version of libinput.  More testing would
> be nice.  :)
> 
> On 29/08/14 01:12 PM, Derek Foreman wrote:
> > If a full screen program is fading out and a touch start happens, it
> > will result in a NULL pointer dereference when weston_touch_set_focus
> > tries to derefernce view->surface->resource.
> > 
> > Instead, this patch sets the focus to NULL, which should be the
> > same as if the program was destroyed during the touch anyway.
> > 
> > An additional test for NULL focus is added to prevent touch bindings
> > from firing without a focus.
> > 
> > Closes bug 78706

I'd prefer:
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=78706

> > ---
> >  src/input.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> > 
> > diff --git a/src/input.c b/src/input.c
> > index 975cd77..bcd603b 100644
> > --- a/src/input.c
> > +++ b/src/input.c
> > @@ -1419,8 +1419,14 @@ weston_touch_set_focus(struct weston_seat *seat, struct weston_view *view)
> >  	}
> >  
> >  	if (view) {
> > -		struct wl_client *surface_client =
> > -			wl_resource_get_client(view->surface->resource);
> > +		struct wl_client *surface_client;
> > +
> > +		if (!view->surface->resource) {
> > +			seat->touch->focus = NULL;
> > +			return;
> > +		}
> > +
> > +		surface_client = wl_resource_get_client(view->surface->resource);

Hi,

this hunk is good.

> >  		move_resources_for_client(focus_resource_list,
> >  					  &seat->touch->resource_list,
> >  					  surface_client);
> > @@ -1479,8 +1485,9 @@ notify_touch(struct weston_seat *seat, uint32_t time, int touch_id,
> >  			return;
> >  		}
> >  
> > -		weston_compositor_run_touch_binding(ec, seat,
> > -						    time, touch_type);
> > +		if (seat->touch->focus)
> > +			weston_compositor_run_touch_binding(ec, seat,
> > +							    time, touch_type);

But this would also make global touch gestures not work. OTOH, we don't
have any global touch gestures in Weston, so it would work for now. The
touch bindings we have are touch_to_activate_binding and
touch_move_binding.

touch_to_activate_binding() already has a check for seat->touch->focus
== NULL, so that shouldn't crash.

touch_move_binding() is missing the check, so it should be added there
instead.

Nothing says a touch binding has to use the focused view, it might as
well change the workspace or something.

Could you move the latter check to touch_move_binding()?


Thanks,
pq