[PATCH libinput] Do not abort on invalid speed.

[Olivier Fourdan]

Hi Peter,

Just to clarify, evdev_accel_config_set_speed() calls filter_set_speed() which calls accelerator_set_speed() which reaches the assert().

My patch basically removes the assert() and replaces it by a return false so that it fails gracefully.

Cheers,
Olivier

----- Original Message -----
From: "Olivier Fourdan" <ofourdan at redhat.com>
To: "Peter Hutterer" <peter.hutterer at who-t.net>
Cc: wayland-devel at lists.freedesktop.org
Sent: Wednesday, 4 February, 2015 10:28:22 PM
Subject: Re: [PATCH libinput] Do not abort on invalid speed.

Hi Peter,

Weird, that occurs with xf86-drv-libinput and libinput both from git as of today.

I don't think this is an uninitialized variable, it's really the Xorg client (read, my code) that sends values out of range and that takes the whole X server down because of the assert()...

Cheers,
Olivier

----- Original Message -----
From: "Peter Hutterer" <peter.hutterer at who-t.net>
To: "Olivier Fourdan" <ofourdan at redhat.com>
Cc: wayland-devel at lists.freedesktop.org
Sent: Wednesday, 4 February, 2015 10:15:56 PM
Subject: Re: [PATCH libinput] Do not abort on invalid speed.

On Wed, Feb 04, 2015 at 10:34:25AM +0100, Olivier Fourdan wrote:
> Libinput's accelerator_set_speed() asserts the given speed value is
> within the [-1.0, 1.0] range.
> 
> When using xf86-input-libinput, a client may try to set an invalid speed
> which will cause the entire Xserver to abort.
> 
> Instead of aborting on invalid speed values, simply return a failure
> (and log a message).
> 
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>

I think this was fixed with b21fbfd947d4bc18b766c46ab1d85a3f8d0977f4 and
199cd87b071f6fd0d4a1065ae3d678552259f883, it was caused by an uninitialized
variable.
https://bugs.freedesktop.org/show_bug.cgi?id=88899

libinput_device_config_accel_set_speed() already has the checks for [-1, 1],
so by the time we get here a speed outside those boundaries is a bug.

Cheers,
   Peter

> ---
>  src/evdev.c  | 5 ++++-
>  src/filter.c | 3 ++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/evdev.c b/src/evdev.c
> index 6e318dc..c96a66b 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1194,8 +1194,11 @@ evdev_accel_config_set_speed(struct libinput_device *device, double speed)
>  {
>  	struct evdev_device *dev = (struct evdev_device *)device;
>  
> -	if (!filter_set_speed(dev->pointer.filter, speed))
> +	if (!filter_set_speed(dev->pointer.filter, speed)) {
> +		log_bug_libinput(device->seat->libinput,
> +				 "Invalid speed %f\n", speed);
>  		return LIBINPUT_CONFIG_STATUS_INVALID;
> +	}
>  
>  	return LIBINPUT_CONFIG_STATUS_SUCCESS;
>  }
> diff --git a/src/filter.c b/src/filter.c
> index 72ef760..91afdcd 100644
> --- a/src/filter.c
> +++ b/src/filter.c
> @@ -260,7 +260,8 @@ accelerator_set_speed(struct motion_filter *filter,
>  	struct pointer_accelerator *accel_filter =
>  		(struct pointer_accelerator *)filter;
>  
> -	assert(speed >= -1.0 && speed <= 1.0);
> +	if (speed < -1.0 || speed > 1.0)
> +		return false;
>  
>  	/* delay when accel kicks in */
>  	accel_filter->threshold = DEFAULT_THRESHOLD - speed/6.0;
> -- 
> 2.1.0
>