Drop invalid ABS_MT_TRACKING_ID changes

Follow-up to
commit 41334b5b40
Author: Peter Hutterer <peter.hutterer@who-t.net>
Date:   Thu Mar 6 11:54:00 2014 +1000

    If the tracking ID changes during SYN_DROPPED, terminate the touch first

In normal mode, we may get double tracking ID events in the same slot, but
only if we either have a user-generated event sequence (uinput) or a malicious
device that tries to send data on a slot > dev->num_slots.
Since the client is unlikely to be able to handle these events, discard the
ABS_MT_TRACKING_ID completely. This is a bug somewhere in the stack, so
complain and hobble on along.

Note: the kernel doesn't allow that, but we cap to num_slots anyway, see
66fee1bec4.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>

libevdev/libevdev.c

@@ -36,6 +36,12 @@
 
 #define MAXEVENTS 64
 
+enum event_filter_status {
+	EVENT_FILTER_NONE,	/**< Event untouched by filters */
+	EVENT_FILTER_MODIFIED,	/**< Event was modified */
+	EVENT_FILTER_DISCARD,	/**< Discard current event */
+};
+
 static int sync_mt_state(struct libevdev *dev, int create_events);
 
 static inline int*
@@ -880,10 +886,11 @@ read_more_events(struct libevdev *dev)
 
 /**
  * Sanitize/modify events where needed.
- * @return 0 if untouched, 1 if modified.
  */
-static inline int
+static inline enum event_filter_status
-sanitize_event(const struct libevdev *dev, struct input_event *ev)
+sanitize_event(const struct libevdev *dev,
+	       struct input_event *ev,
+	       enum SyncState sync_state)
 {
 	if (unlikely(dev->num_slots > -1 &&
 		     libevdev_event_is_code(ev, EV_ABS, ABS_MT_SLOT) &&
@@ -892,16 +899,32 @@ sanitize_event(const struct libevdev *dev, struct input_event *ev)
 				"Capping to announced max slot number %d.\n",
 				dev->name, ev->value, dev->num_slots - 1);
 		ev->value = dev->num_slots - 1;
-		return 1;
+		return EVENT_FILTER_MODIFIED;
+
+	/* Drop any invalid tracking IDs, they are only supposed to go from
+	   N to -1 or from -1 to N. Never from -1 to -1, or N to M. Very
+	   unlikely to ever happen from a real device.
+	   */
+	} else if (unlikely(sync_state == SYNC_NONE &&
+			    dev->num_slots > -1 &&
+			    libevdev_event_is_code(ev, EV_ABS, ABS_MT_TRACKING_ID) &&
+			    ((ev->value == -1 &&
+			     *slot_value(dev, dev->current_slot, ABS_MT_TRACKING_ID) == -1) ||
+			     (ev->value != -1 &&
+			     *slot_value(dev, dev->current_slot, ABS_MT_TRACKING_ID) != -1)))) {
+		log_bug("Device \"%s\" received a double tracking ID %d in slot %d.\n",
+			dev->name, ev->value, dev->current_slot);
+		return EVENT_FILTER_DISCARD;
 	}
 
-	return 0;
+	return EVENT_FILTER_NONE;
 }
 
 LIBEVDEV_EXPORT int
 libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event *ev)
 {
 	int rc = LIBEVDEV_READ_STATUS_SUCCESS;
+	enum event_filter_status filter_status;
 
 	if (!dev->initialized) {
 		log_bug("device not initialized. call libevdev_set_fd() first\n");
@@ -934,8 +957,8 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
 		   of the device too */
 		while (queue_shift(dev, &e) == 0) {
 			dev->queue_nsync--;
-			sanitize_event(dev, &e);
+			if (sanitize_event(dev, &e, dev->sync_state) != EVENT_FILTER_DISCARD)
-			update_state(dev, &e);
+				update_state(dev, &e);
 		}
 
 		dev->sync_state = SYNC_NONE;
@@ -965,11 +988,13 @@ libevdev_next_event(struct libevdev *dev, unsigned int flags, struct input_event
 		if (queue_shift(dev, ev) != 0)
 			return -EAGAIN;
 
-		sanitize_event(dev, ev);
+		filter_status = sanitize_event(dev, ev, dev->sync_state);
-		update_state(dev, ev);
+		if (filter_status != EVENT_FILTER_DISCARD)
+			update_state(dev, ev);
 
 	/* if we disabled a code, get the next event instead */
-	} while(!libevdev_has_event_code(dev, ev->type, ev->code));
+	} while(filter_status == EVENT_FILTER_DISCARD ||
+		!libevdev_has_event_code(dev, ev->type, ev->code));
 
 	rc = LIBEVDEV_READ_STATUS_SUCCESS;
 	if (ev->type == EV_SYN && ev->code == SYN_DROPPED) {
@@ -1163,7 +1188,7 @@ libevdev_set_event_value(struct libevdev *dev, unsigned int type, unsigned int c
 	e.code = code;
 	e.value = value;
 
-	if (sanitize_event(dev, &e))
+	if (sanitize_event(dev, &e, SYNC_NONE) != EVENT_FILTER_NONE)
 		return -1;
 
 	switch(type) {

test/test-libevdev-events.c

@@ -1424,6 +1424,164 @@ START_TEST(test_mt_slot_ranges_invalid)
 }
 END_TEST
 
+START_TEST(test_mt_tracking_id_discard)
+{
+	struct uinput_device* uidev;
+	struct libevdev *dev;
+	int rc;
+	struct input_event ev;
+	struct input_absinfo abs[6];
+
+	memset(abs, 0, sizeof(abs));
+	abs[0].value = ABS_X;
+	abs[0].maximum = 1000;
+	abs[1].value = ABS_MT_POSITION_X;
+	abs[1].maximum = 1000;
+
+	abs[2].value = ABS_Y;
+	abs[2].maximum = 1000;
+	abs[3].value = ABS_MT_POSITION_Y;
+	abs[3].maximum = 1000;
+
+	abs[4].value = ABS_MT_SLOT;
+	abs[4].maximum = 10;
+	abs[5].value = ABS_MT_TRACKING_ID;
+	abs[5].maximum = 500;
+
+	rc = test_create_abs_device(&uidev, &dev,
+				    6, abs,
+				    EV_SYN, SYN_REPORT,
+				    -1);
+
+	uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 1);
+	uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1);
+	uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+	/* second tracking ID on same slot */
+	uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 2);
+	uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+	libevdev_set_log_function(test_logfunc_ignore_error, NULL);
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_ABS);
+	ck_assert_int_eq(ev.code, ABS_MT_SLOT);
+	ck_assert_int_eq(ev.value, 1);
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_ABS);
+	ck_assert_int_eq(ev.code, ABS_MT_TRACKING_ID);
+	ck_assert_int_eq(ev.value, 1);
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_SYN);
+	ck_assert_int_eq(ev.code, SYN_REPORT);
+	ck_assert_int_eq(ev.value, 0);
+
+	/* expect tracking ID discarded */
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_SYN);
+	ck_assert_int_eq(ev.code, SYN_REPORT);
+	ck_assert_int_eq(ev.value, 0);
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, -EAGAIN);
+
+	libevdev_set_log_function(test_logfunc_abort_on_error, NULL);
+
+	uinput_device_free(uidev);
+	libevdev_free(dev);
+}
+END_TEST
+
+START_TEST(test_mt_tracking_id_discard_neg_1)
+{
+	struct uinput_device* uidev;
+	struct libevdev *dev;
+	int rc;
+	struct input_event ev;
+	struct input_absinfo abs[6];
+	int pipefd[2];
+	struct input_event events[] = {
+		{ .type = EV_ABS, .code = ABS_MT_TRACKING_ID, .value = -1 },
+		{ .type = EV_SYN, .code = SYN_REPORT, .value = 0 },
+	};
+
+	rc = pipe2(pipefd, O_NONBLOCK);
+	ck_assert_int_eq(rc, 0);
+
+	memset(abs, 0, sizeof(abs));
+	abs[0].value = ABS_X;
+	abs[0].maximum = 1000;
+	abs[1].value = ABS_MT_POSITION_X;
+	abs[1].maximum = 1000;
+
+	abs[2].value = ABS_Y;
+	abs[2].maximum = 1000;
+	abs[3].value = ABS_MT_POSITION_Y;
+	abs[3].maximum = 1000;
+
+	abs[4].value = ABS_MT_SLOT;
+	abs[4].maximum = 10;
+	abs[5].value = ABS_MT_TRACKING_ID;
+	abs[5].maximum = 500;
+
+	rc = test_create_abs_device(&uidev, &dev,
+				    6, abs,
+				    EV_SYN, SYN_REPORT,
+				    -1);
+	uinput_device_event(uidev, EV_ABS, ABS_MT_SLOT, 1);
+	uinput_device_event(uidev, EV_ABS, ABS_MT_TRACKING_ID, 1);
+	uinput_device_event(uidev, EV_SYN, SYN_REPORT, 0);
+
+	while (libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev) != -EAGAIN)
+		;
+
+	libevdev_set_log_function(test_logfunc_ignore_error, NULL);
+
+	/* two -1 tracking ids, need to use the pipe here, the kernel will
+	   filter it otherwise */
+	libevdev_change_fd(dev, pipefd[0]);
+
+	rc = write(pipefd[1], events, sizeof(events));
+	ck_assert_int_eq(rc, sizeof(events));
+	rc = write(pipefd[1], events, sizeof(events));
+	ck_assert_int_eq(rc, sizeof(events));
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_ABS);
+	ck_assert_int_eq(ev.code, ABS_MT_TRACKING_ID);
+	ck_assert_int_eq(ev.value, -1);
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_SYN);
+	ck_assert_int_eq(ev.code, SYN_REPORT);
+	ck_assert_int_eq(ev.value, 0);
+
+	/* expect second tracking ID discarded */
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, LIBEVDEV_READ_STATUS_SUCCESS);
+	ck_assert_int_eq(ev.type, EV_SYN);
+	ck_assert_int_eq(ev.code, SYN_REPORT);
+	ck_assert_int_eq(ev.value, 0);
+
+	rc = libevdev_next_event(dev, LIBEVDEV_READ_FLAG_NORMAL, &ev);
+	ck_assert_int_eq(rc, -EAGAIN);
+
+	libevdev_set_log_function(test_logfunc_abort_on_error, NULL);
+
+	uinput_device_free(uidev);
+	libevdev_free(dev);
+}
+END_TEST
+
 START_TEST(test_ev_rep_values)
 {
 	struct uinput_device* uidev;
@@ -1719,6 +1877,8 @@ libevdev_events(void)
 	tcase_add_test(tc, test_mt_event_values);
 	tcase_add_test(tc, test_mt_event_values_invalid);
 	tcase_add_test(tc, test_mt_slot_ranges_invalid);
+	tcase_add_test(tc, test_mt_tracking_id_discard);
+	tcase_add_test(tc, test_mt_tracking_id_discard_neg_1);
 	tcase_add_test(tc, test_ev_rep_values);
 	suite_add_tcase(s, tc);