[PATCH] HID: multitouch: bound the slot index before touching mt_io_flags

[Your Name <qwe.aldo@gmail.com>]

From: Aldo Ariel Panzardo <qwe.aldo@gmail.com>

td->mt_io_flags is a single unsigned long.  Its first MT_IO_SLOTS_BITS
bits track per-slot state, read back through MT_IO_SLOTS_MASK, while bit
32 holds MT_IO_FLAGS_RUNNING, which the report path and the sticky-finger
timer take and release with test_and_set_bit_lock() and
clear_bit_unlock().

set_bit()/clear_bit() are called on that word with the slot number as the
bit index:

	set_bit(slotnum, &td->mt_io_flags);

slotnum is bounded only by td->maxcontacts, which is taken from the HID
CONTACTMAX feature value and can reach MT_MAX_MAXCONTACT (250).  Two
things go wrong once a device reports a slot number of 8 or more:

 - a slot number of 32 sets MT_IO_FLAGS_RUNNING from the data path.
   mt_expired_timeout() then finds the flag already set and returns
   early, so sticky fingers are never released, and the
   clear_bit_unlock() in the other path drops a lock it does not hold.
   Several in-tree classes declare .maxcontacts of 40 and 60, so this is
   reachable with ordinary hardware.

 - a slot number of BITS_PER_LONG or more writes past the end of the word
   altogether, corrupting the struct mt_device fields that follow it.

The same unchecked index is used in mt_release_pending_palms(), where
slotnum comes from a for_each_set_bit() bounded by td->maxcontacts, and
in mt_release_contacts(), where it is bounded by mt->num_slots.

Bound the index to the reserved slot range in one place and use that for
every set_bit()/clear_bit() on mt_io_flags.  Dropping the out-of-range
slots is not a functional change: both readers of the slot state mask
with MT_IO_SLOTS_MASK, so bits at or above MT_IO_SLOTS_BITS were never
observable to begin with.

Also correct the comment on the field, which claimed that eight bits were
sufficient because at most 250 slots are supported.

Fixes: 46f781e0d151 ("HID: multitouch: fix sticky fingers")
Cc: stable@vger.kernel.org
Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
---
 drivers/hid/hid-multitouch.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 0495152091e3..463eab9f73cd 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -98,6 +98,7 @@ enum report_mode {
 };
 
 #define MT_IO_SLOTS_MASK		GENMASK(7, 0) /* reserve first 8 bits for slot tracking */
+#define MT_IO_SLOTS_BITS		8 /* bits covered by MT_IO_SLOTS_MASK */
 #define MT_IO_FLAGS_RUNNING		32
 
 static const bool mtrue = true;		/* default for true */
@@ -175,9 +176,10 @@ struct mt_device {
 	struct hid_haptic_device *haptic;	/* haptic related configuration */
 	struct hid_device *hdev;	/* hid_device we're attached to */
 	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_RUNNING)
-					 * first 8 bits are reserved for keeping the slot
-					 * states, this is fine because we only support up
-					 * to 250 slots (MT_MAX_MAXCONTACT)
+					 * the first MT_IO_SLOTS_BITS bits are reserved
+					 * for keeping the slot states; higher slot
+					 * numbers are not tracked here, see
+					 * mt_io_slot_set()
 					 */
 	__u8 inputmode_value;	/* InputMode HID feature value */
 	__u8 maxcontacts;
@@ -1027,6 +1029,25 @@ static int mt_compute_slot(struct mt_dev
 	return input_mt_get_slot_by_key(input, *slot->contactid);
 }
 
+/*
+ * Only the first MT_IO_SLOTS_BITS bits of mt_io_flags track slot state; the
+ * rest of the word holds unrelated flags such as MT_IO_FLAGS_RUNNING.  Slot
+ * numbers are bounded by td->maxcontacts, which is taken from the HID
+ * CONTACTMAX feature and can be much larger, so the index has to be checked
+ * before touching the word.
+ */
+static void mt_io_slot_set(struct mt_device *td, int slotnum)
+{
+	if (slotnum < MT_IO_SLOTS_BITS)
+		set_bit(slotnum, &td->mt_io_flags);
+}
+
+static void mt_io_slot_clear(struct mt_device *td, int slotnum)
+{
+	if (slotnum < MT_IO_SLOTS_BITS)
+		clear_bit(slotnum, &td->mt_io_flags);
+}
+
 static void mt_release_pending_palms(struct mt_device *td,
 				     struct mt_application *app,
 				     struct input_dev *input)
@@ -1036,7 +1057,7 @@ static void mt_release_pending_palms(str
 
 	for_each_set_bit(slotnum, app->pending_palm_slots, td->maxcontacts) {
 		clear_bit(slotnum, app->pending_palm_slots);
-		clear_bit(slotnum, &td->mt_io_flags);
+		mt_io_slot_clear(td, slotnum);
 
 		input_mt_slot(input, slotnum);
 		input_mt_report_slot_inactive(input);
@@ -1247,9 +1268,9 @@ static int mt_process_slot(struct mt_dev
 		input_event(input, EV_ABS, ABS_MT_TOUCH_MAJOR, major);
 		input_event(input, EV_ABS, ABS_MT_TOUCH_MINOR, minor);
 
-		set_bit(slotnum, &td->mt_io_flags);
+		mt_io_slot_set(td, slotnum);
 	} else {
-		clear_bit(slotnum, &td->mt_io_flags);
+		mt_io_slot_clear(td, slotnum);
 	}
 
 	return 0;
@@ -2062,7 +2083,7 @@ static void mt_release_contacts(struct h
 			for (i = 0; i < mt->num_slots; i++) {
 				input_mt_slot(input_dev, i);
 				input_mt_report_slot_inactive(input_dev);
-				clear_bit(i, &td->mt_io_flags);
+				mt_io_slot_clear(td, i);
 			}
 			input_mt_sync_frame(input_dev);
 			input_sync(input_dev);
-- 
2.43.0