Drop invalid ABS_MT_TRACKING_ID changes

Follow-up to
commit 41334b5b40cd5456f5f584b55d8888aaafa1f26e
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
66fee1bec4c4b021e1b54adcd775cf6e2aa84869.

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

diff --git a/libevdev/libevdev.c b/libevdev/libevdev.c
index 119f18f..96bd41b 100644 (file)
--- a/libevdev/libevdev.c
+++ b/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
-sanitize_event(const struct libevdev *dev, struct input_event *ev)
+static inline enum event_filter_status
+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);
-                       update_state(dev, &e);
+                       if (sanitize_event(dev, &e, dev->sync_state) != EVENT_FILTER_DISCARD)
+                               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);
-               update_state(dev, ev);
+               filter_status = sanitize_event(dev, ev, dev->sync_state);
+               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) {

diff --git a/test/test-libevdev-events.c b/test/test-libevdev-events.c
index fda7617..dc412a7 100644 (file)
--- a/test/test-libevdev-events.c
+++ b/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);