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

[Derek Foreman]

On 04/09/14 06:49 AM, Pekka Paalanen wrote:
> 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

Ok

>>> ---
>>>  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()?

I've re-checked and the segfault that hunk was trying to stop was indeed
in touch_move_binding().

Will send a fixed patch shortly.

> Thanks,
> pq
> 

Thanks,
Derek