[PATCH libinput] evdev: reject devices with only one of x/y resolution

[Hans de Goede]

Hi,

On 08-04-15 23:51, Peter Hutterer wrote:
> This is a kernel bug, reject such devices outright. This saves us from a bunch
> of extra double checks to make sure that the resolutions are always set up.
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>

Looks good, I've added my Rev-by and pushed this one.

Regards,

Hans

> ---
>   src/evdev.c   | 32 +++++++++++++++++-----
>   test/device.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 112 insertions(+), 7 deletions(-)
>
> diff --git a/src/evdev.c b/src/evdev.c
> index a6d6fae..243cd22 100644
> --- a/src/evdev.c
> +++ b/src/evdev.c
> @@ -1377,13 +1377,6 @@ evdev_fix_abs_resolution(struct evdev_device *device,
>   	absx = libevdev_get_abs_info(evdev, xcode);
>   	absy = libevdev_get_abs_info(evdev, ycode);
>
> -	if ((absx->resolution == 0 && absy->resolution != 0) ||
> -	    (absx->resolution != 0 && absy->resolution == 0)) {
> -		log_bug_kernel(libinput,
> -			       "Kernel has only x or y resolution, not both.\n");
> -		return 0;
> -	}
> -
>   	if (absx->resolution == 0 || absx->resolution == EVDEV_FAKE_RESOLUTION) {
>   		fixed = *absx;
>   		fixed.resolution = xresolution;
> @@ -1487,8 +1480,10 @@ evdev_check_min_max(struct evdev_device *device, unsigned int code)
>   static int
>   evdev_reject_device(struct evdev_device *device)
>   {
> +	struct libinput *libinput = device->base.seat->libinput;
>   	struct libevdev *evdev = device->evdev;
>   	unsigned int code;
> +	const struct input_absinfo *absx, *absy;
>
>   	if (libevdev_has_event_code(evdev, EV_ABS, ABS_X) ^
>   	    libevdev_has_event_code(evdev, EV_ABS, ABS_Y))
> @@ -1498,6 +1493,29 @@ evdev_reject_device(struct evdev_device *device)
>   	    libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_Y))
>   		return -1;
>
> +	if (libevdev_has_event_code(evdev, EV_ABS, ABS_X)) {
> +		absx = libevdev_get_abs_info(evdev, ABS_X);
> +		absy = libevdev_get_abs_info(evdev, ABS_Y);
> +		if ((absx->resolution == 0 && absy->resolution != 0) ||
> +		    (absx->resolution != 0 && absy->resolution == 0)) {
> +			log_bug_kernel(libinput,
> +				       "Kernel has only x or y resolution, not both.\n");
> +			return -1;
> +		}
> +	}
> +
> +	if (!evdev_is_fake_mt_device(device) &&
> +	    libevdev_has_event_code(evdev, EV_ABS, ABS_MT_POSITION_X)) {
> +		absx = libevdev_get_abs_info(evdev, ABS_MT_POSITION_X);
> +		absy = libevdev_get_abs_info(evdev, ABS_MT_POSITION_Y);
> +		if ((absx->resolution == 0 && absy->resolution != 0) ||
> +		    (absx->resolution != 0 && absy->resolution == 0)) {
> +			log_bug_kernel(libinput,
> +				       "Kernel has only x or y MT resolution, not both.\n");
> +			return -1;
> +		}
> +	}
> +
>   	for (code = 0; code < ABS_CNT; code++) {
>   		switch (code) {
>   		case ABS_MISC:
> diff --git a/test/device.c b/test/device.c
> index c9d5255..cf9885a 100644
> --- a/test/device.c
> +++ b/test/device.c
> @@ -894,6 +894,91 @@ START_TEST(abs_mt_device_no_range)
>   }
>   END_TEST
>
> +START_TEST(abs_device_missing_res)
> +{
> +	struct libevdev_uinput *uinput;
> +	struct libinput *li;
> +	struct libinput_device *device;
> +	struct input_absinfo absinfo[] = {
> +		{ ABS_X, 0, 10, 0, 0, 10 },
> +		{ ABS_Y, 0, 10, 0, 0, 0 },
> +		{ -1, -1, -1, -1, -1, -1 }
> +	};
> +
> +	li = litest_create_context();
> +	litest_disable_log_handler(li);
> +	uinput = litest_create_uinput_abs_device("test device", NULL,
> +						 absinfo,
> +						 EV_KEY, BTN_LEFT,
> +						 EV_KEY, BTN_RIGHT,
> +						 -1);
> +	device = libinput_path_add_device(li,
> +					  libevdev_uinput_get_devnode(uinput));
> +	ck_assert(device == NULL);
> +	libevdev_uinput_destroy(uinput);
> +
> +	absinfo[0].resolution = 0;
> +	absinfo[1].resolution = 20;
> +	uinput = litest_create_uinput_abs_device("test device", NULL,
> +						 absinfo,
> +						 EV_KEY, BTN_LEFT,
> +						 EV_KEY, BTN_RIGHT,
> +						 -1);
> +	device = libinput_path_add_device(li,
> +					  libevdev_uinput_get_devnode(uinput));
> +	ck_assert(device == NULL);
> +	libevdev_uinput_destroy(uinput);
> +
> +	litest_restore_log_handler(li);
> +	libinput_unref(li);
> +
> +}
> +END_TEST
> +
> +START_TEST(abs_mt_device_missing_res)
> +{
> +	struct libevdev_uinput *uinput;
> +	struct libinput *li;
> +	struct libinput_device *device;
> +	struct input_absinfo absinfo[] = {
> +		{ ABS_X, 0, 10, 0, 0, 10 },
> +		{ ABS_Y, 0, 10, 0, 0, 10 },
> +		{ ABS_MT_SLOT, 0, 2, 0, 0, 0 },
> +		{ ABS_MT_TRACKING_ID, 0, 255, 0, 0, 0 },
> +		{ ABS_MT_POSITION_X, 0, 10, 0, 0, 10 },
> +		{ ABS_MT_POSITION_Y, 0, 10, 0, 0, 0 },
> +		{ -1, -1, -1, -1, -1, -1 }
> +	};
> +
> +	li = litest_create_context();
> +	litest_disable_log_handler(li);
> +	uinput = litest_create_uinput_abs_device("test device", NULL,
> +						 absinfo,
> +						 EV_KEY, BTN_LEFT,
> +						 EV_KEY, BTN_RIGHT,
> +						 -1);
> +	device = libinput_path_add_device(li,
> +					  libevdev_uinput_get_devnode(uinput));
> +	ck_assert(device == NULL);
> +	libevdev_uinput_destroy(uinput);
> +
> +	absinfo[4].resolution = 0;
> +	absinfo[5].resolution = 20;
> +	uinput = litest_create_uinput_abs_device("test device", NULL,
> +						 absinfo,
> +						 EV_KEY, BTN_LEFT,
> +						 EV_KEY, BTN_RIGHT,
> +						 -1);
> +	device = libinput_path_add_device(li,
> +					  libevdev_uinput_get_devnode(uinput));
> +	ck_assert(device == NULL);
> +	libevdev_uinput_destroy(uinput);
> +
> +	litest_restore_log_handler(li);
> +	libinput_unref(li);
> +
> +}
> +END_TEST
>   int main (int argc, char **argv)
>   {
>   	litest_add("device:sendevents", device_sendevents_config, LITEST_ANY, LITEST_TOUCHPAD);
> @@ -928,6 +1013,8 @@ int main (int argc, char **argv)
>   	litest_add_no_device("device:invalid devices", abs_mt_device_no_absy);
>   	litest_add_no_device("device:invalid devices", abs_device_no_range);
>   	litest_add_no_device("device:invalid devices", abs_mt_device_no_range);
> +	litest_add_no_device("device:invalid devices", abs_device_missing_res);
> +	litest_add_no_device("device:invalid devices", abs_mt_device_missing_res);
>
>   	return litest_run(argc, argv);
>   }
>