Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags
       [not found] <CAO-hwJLS6uAX__aGkmpcDQ8v6GJaHGrL+TOYXVRX6jvjzNW+wg@mail.gmail.com>
@ 2026-07-01 17:13 ` Trung Nguyen
  2026-07-01 17:13   ` [PATCH 2/2] selftests/hid: multitouch: test a large ContactCountMaximum Trung Nguyen
  2026-07-01 19:12   ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Benjamin Tissoires
  0 siblings, 2 replies; 3+ messages in thread
From: Trung Nguyen @ 2026-07-01 17:13 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: linux-input, linux-kernel, Trung Nguyen, stable

mt_io_flags is a single unsigned long, but mt_process_slot(),
mt_release_pending_palms() and mt_release_contacts() use it as a
per-slot bitmap indexed by the slot number. That slot number is only
bounded by td->maxcontacts, which is taken from the device's
ContactCountMaximum feature report and can be up to 255, not by
BITS_PER_LONG.

As a result, a multitouch device that advertises a large contact count
makes set_bit()/clear_bit() operate past the mt_io_flags word and
corrupt the adjacent members of struct mt_device. The sticky-fingers
release timer is the easiest way to reach this. mt_release_contacts()
runs

	for (i = 0; i < mt->num_slots; i++)
		clear_bit(i, &td->mt_io_flags);

with num_slots == maxcontacts. For maxcontacts around 250 the loop
clears the bits that overlap td->applications.next, zeroing that list
head, and the list_for_each_entry() that immediately follows then
dereferences NULL. The kernel panics from timer (softirq) context. On a
KASAN build this shows up as a general protection fault in
mt_release_contacts() with a null-ptr-deref at offset 0x58, which is
offsetof(struct mt_application, num_received).

The state is reachable from an untrusted USB or Bluetooth HID
multitouch device; no local privileges are required.

Store the per-slot active state in a separately allocated bitmap sized
for maxcontacts, the same pattern already used for pending_palm_slots,
and keep only MT_IO_FLAGS_RUNNING in mt_io_flags. The two
"mt_io_flags & MT_IO_SLOTS_MASK" arming checks become
bitmap_empty(td->active_slots, td->maxcontacts).

Move MT_IO_FLAGS_RUNNING back to bit 0. It was bumped to bit 32 by the
same commit to leave the low byte for the slot bits; with the slot bits
gone it fits in bit 0 again, which also keeps it within the unsigned
long on 32-bit.

Fixes: 46f781e0d151 ("HID: multitouch: fix sticky fingers")
Cc: stable@vger.kernel.org
Signed-off-by: Trung Nguyen <trungnh@cystack.net>
---
 drivers/hid/hid-multitouch.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 0495152091e3..edb37b4c867e 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -31,6 +31,7 @@
  * [1] https://gitlab.freedesktop.org/libevdev/hid-tools
  */
 
+#include <linux/bitmap.h>
 #include <linux/bits.h>
 #include <linux/device.h>
 #include <linux/hid.h>
@@ -97,8 +98,7 @@ enum report_mode {
 	TOUCHPAD_REPORT_ALL = TOUCHPAD_REPORT_BUTTONS | TOUCHPAD_REPORT_CONTACTS,
 };
 
-#define MT_IO_SLOTS_MASK		GENMASK(7, 0) /* reserve first 8 bits for slot tracking */
-#define MT_IO_FLAGS_RUNNING		32
+#define MT_IO_FLAGS_RUNNING		0
 
 static const bool mtrue = true;		/* default for true */
 static const bool mfalse;		/* default for false */
@@ -174,10 +174,9 @@ struct mt_device {
 	struct timer_list release_timer;	/* to release sticky fingers */
 	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)
+	unsigned long mt_io_flags;	/* mt flags (MT_IO_FLAGS_RUNNING) */
+	unsigned long *active_slots;	/* bitmap of slots with an active
+					 * contact, sized for maxcontacts
 					 */
 	__u8 inputmode_value;	/* InputMode HID feature value */
 	__u8 maxcontacts;
@@ -1036,7 +1035,7 @@ static void mt_release_pending_palms(struct mt_device *td,
 
 	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);
+		clear_bit(slotnum, td->active_slots);
 
 		input_mt_slot(input, slotnum);
 		input_mt_report_slot_inactive(input);
@@ -1247,9 +1246,9 @@ static int mt_process_slot(struct mt_device *td, struct input_dev *input,
 		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);
+		set_bit(slotnum, td->active_slots);
 	} else {
-		clear_bit(slotnum, &td->mt_io_flags);
+		clear_bit(slotnum, td->active_slots);
 	}
 
 	return 0;
@@ -1384,7 +1383,7 @@ static void mt_touch_report(struct hid_device *hid,
 	 * defect.
 	 */
 	if (app->quirks & MT_QUIRK_STICKY_FINGERS) {
-		if (td->mt_io_flags & MT_IO_SLOTS_MASK)
+		if (!bitmap_empty(td->active_slots, td->maxcontacts))
 			mod_timer(&td->release_timer,
 				  jiffies + msecs_to_jiffies(100));
 		else
@@ -1443,6 +1442,15 @@ static int mt_touch_input_configured(struct hid_device *hdev,
 	if (td->is_pressurepad)
 		__set_bit(INPUT_PROP_PRESSUREPAD, input->propbit);
 
+	if (!td->active_slots) {
+		td->active_slots = devm_kcalloc(&td->hdev->dev,
+						BITS_TO_LONGS(td->maxcontacts),
+						sizeof(long),
+						GFP_KERNEL);
+		if (!td->active_slots)
+			return -ENOMEM;
+	}
+
 	app->pending_palm_slots = devm_kcalloc(&hi->input->dev,
 					       BITS_TO_LONGS(td->maxcontacts),
 					       sizeof(long),
@@ -2062,7 +2070,7 @@ static void mt_release_contacts(struct hid_device *hid)
 			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);
+				clear_bit(i, td->active_slots);
 			}
 			input_mt_sync_frame(input_dev);
 			input_sync(input_dev);
@@ -2085,7 +2093,7 @@ static void mt_expired_timeout(struct timer_list *t)
 	 */
 	if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
 		return;
-	if (td->mt_io_flags & MT_IO_SLOTS_MASK)
+	if (!bitmap_empty(td->active_slots, td->maxcontacts))
 		mt_release_contacts(hdev);
 	clear_bit_unlock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags);
 }
-- 
2.45.1.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* [PATCH 2/2] selftests/hid: multitouch: test a large ContactCountMaximum
  2026-07-01 17:13 ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Trung Nguyen
@ 2026-07-01 17:13   ` Trung Nguyen
  2026-07-01 19:12   ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Benjamin Tissoires
  1 sibling, 0 replies; 3+ messages in thread
From: Trung Nguyen @ 2026-07-01 17:13 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: linux-input, linux-kernel, Trung Nguyen

Add a regression test for the out-of-bounds bit operations on
struct mt_device.mt_io_flags.

A HID multitouch device can advertise a ContactCountMaximum far larger
than the number of contacts a single report describes, up to 255. The
driver used to keep the per-slot active state in the bits of a single
unsigned long and index set_bit()/clear_bit() by the slot number, so such
a device drove those operations out of bounds. The sticky-fingers release
timer made it fatal: mt_release_contacts() cleared one bit per slot and
overwrote the adjacent members of struct mt_device.

The new device advertises a ContactCountMaximum of 250 while exposing only
a few finger collections (a large contact count cannot be expressed with
one finger collection per contact within the HID descriptor size limit).
The test sends a single contact and lets the 100ms sticky-fingers timer
release it. A kernel without the fix panics in mt_release_contacts(); a
fixed kernel reports the release cleanly.

Signed-off-by: Trung Nguyen <trungnh@cystack.net>
---
 .../selftests/hid/tests/test_multitouch.py    | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/tools/testing/selftests/hid/tests/test_multitouch.py b/tools/testing/selftests/hid/tests/test_multitouch.py
index fa4fb2054bd4..7897340118b4 100644
--- a/tools/testing/selftests/hid/tests/test_multitouch.py
+++ b/tools/testing/selftests/hid/tests/test_multitouch.py
@@ -513,6 +513,79 @@ class SmartTechDigitizer(Digitizer):
         return absinfo is not None and absinfo.resolution == 3
 
 
+class MinWin8TSParallelBigContactMax(Digitizer):
+    """A parallel Win8 touchscreen that advertises a ContactCountMaximum much
+    larger than the number of contacts it actually reports.
+
+    Such firmware makes the driver allocate that many input slots (up to 255)
+    while the input report only carries a few contacts. This is what used to
+    drive the per-slot bit operations on mt_io_flags out of bounds. The number
+    of contacts a HID report can describe is limited by the descriptor size,
+    so a large ContactCountMaximum can only be expressed this way, decoupled
+    from the number of finger collections."""
+
+    def __init__(self, n_fingers=5, contact_max=250):
+        self.phys_max = 120, 90
+        rdesc_finger_str = f"""
+            Usage Page (Digitizers)
+            Usage (Finger)
+            Collection (Logical)
+             Report Size (1)
+             Report Count (1)
+             Logical Minimum (0)
+             Logical Maximum (1)
+             Usage (Tip Switch)
+             Input (Data,Var,Abs)
+             Report Size (7)
+             Logical Maximum (127)
+             Input (Cnst,Var,Abs)
+             Report Size (8)
+             Logical Maximum (255)
+             Usage (Contact Id)
+             Input (Data,Var,Abs)
+             Report Size (16)
+             Unit Exponent (-1)
+             Unit (SILinear: cm)
+             Logical Maximum (4095)
+             Physical Minimum (0)
+             Physical Maximum ({self.phys_max[0]})
+             Usage Page (Generic Desktop)
+             Usage (X)
+             Input (Data,Var,Abs)
+             Physical Maximum ({self.phys_max[1]})
+             Usage (Y)
+             Input (Data,Var,Abs)
+            End Collection
+"""
+        rdesc_str = f"""
+           Usage Page (Digitizers)
+           Usage (Touch Screen)
+           Collection (Application)
+            Report ID (1)
+            {rdesc_finger_str * n_fingers}
+            Unit Exponent (-4)
+            Unit (SILinear: s)
+            Logical Maximum (65535)
+            Physical Maximum (65535)
+            Usage Page (Digitizers)
+            Usage (Scan Time)
+            Input (Data,Var,Abs)
+            Report Size (8)
+            Logical Maximum (255)
+            Usage (Contact Count)
+            Input (Data,Var,Abs)
+            Report ID (2)
+            Logical Maximum ({contact_max})
+            Usage (Contact Max)
+            Feature (Data,Var,Abs)
+          End Collection
+          {Digitizer.msCertificationBlob(68)}
+"""
+        super().__init__(
+            f"uhid test parallel big contact max {contact_max}", rdesc_str
+        )
+
+
 class BaseTest:
     class TestMultitouch(base.BaseTestCase.TestUhid):
         kernel_modules = [KERNEL_MODULE]
@@ -1735,6 +1808,47 @@ class TestMinWin8TSParallel(BaseTest.TestWin8Multitouch):
         return MinWin8TSParallel(10)
 
 
+class TestMinWin8TSParallelBigContactMax(base.BaseTestCase.TestUhid):
+    """Regression test for the out-of-bounds bit operations on
+    struct mt_device.mt_io_flags.
+
+    A Win8 touchscreen may advertise a ContactCountMaximum much larger than
+    the number of contacts it reports. The driver used to keep the per-slot
+    active state in the bits of a single unsigned long while indexing
+    set_bit()/clear_bit() by the slot number, so such a device drove those bit
+    operations out of bounds. The sticky-fingers release timer made it fatal:
+    mt_release_contacts() cleared one bit per slot, overwrote the adjacent
+    struct mt_device members and panicked the kernel.
+
+    Send a single contact, let the 100ms sticky-fingers timer release it, and
+    check that the kernel reports the release cleanly instead of crashing."""
+
+    kernel_modules = [KERNEL_MODULE]
+
+    def create_device(self):
+        return MinWin8TSParallelBigContactMax()
+
+    def test_sticky_fingers_release_big_contact_max(self):
+        uhdev = self.uhdev
+        evdev = uhdev.get_evdev()
+
+        assert evdev.num_slots == uhdev.max_contacts
+
+        t0 = Touch(1, 5, 10)
+        r = uhdev.event([t0])
+        events = uhdev.next_sync_events()
+        self.debug_reports(r, uhdev, events)
+        assert evdev.slots[0][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == 0
+
+        # do not release the contact; the sticky-fingers timer must do it
+        # after 100ms, which is where the out-of-bounds release used to hit
+        time.sleep(0.2)
+        events = uhdev.next_sync_events()
+        self.debug_reports(r, uhdev, events)
+        assert libevdev.InputEvent(libevdev.EV_KEY.BTN_TOUCH, 0) in events
+        assert evdev.slots[0][libevdev.EV_ABS.ABS_MT_TRACKING_ID] == -1
+
+
 class TestMinWin8TSHybrid(BaseTest.TestWin8Multitouch):
     def create_device(self):
         return MinWin8TSHybrid()
-- 
2.45.1.windows.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags
  2026-07-01 17:13 ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Trung Nguyen
  2026-07-01 17:13   ` [PATCH 2/2] selftests/hid: multitouch: test a large ContactCountMaximum Trung Nguyen
@ 2026-07-01 19:12   ` Benjamin Tissoires
  1 sibling, 0 replies; 3+ messages in thread
From: Benjamin Tissoires @ 2026-07-01 19:12 UTC (permalink / raw)
  To: jikos, Trung Nguyen; +Cc: linux-input, linux-kernel, stable

On Thu, 02 Jul 2026 00:13:19 +0700, Trung Nguyen wrote:
> mt_io_flags is a single unsigned long, but mt_process_slot(),
> mt_release_pending_palms() and mt_release_contacts() use it as a
> per-slot bitmap indexed by the slot number. That slot number is only
> bounded by td->maxcontacts, which is taken from the device's
> ContactCountMaximum feature report and can be up to 255, not by
> BITS_PER_LONG.
> 
> [...]

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git (for-7.2/upstream-fixes), thanks!

[1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags
      https://git.kernel.org/hid/hid/c/8813b0612275
[2/2] selftests/hid: multitouch: test a large ContactCountMaximum
      https://git.kernel.org/hid/hid/c/b6eb022890c7

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-07-01 19:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CAO-hwJLS6uAX__aGkmpcDQ8v6GJaHGrL+TOYXVRX6jvjzNW+wg@mail.gmail.com>
2026-07-01 17:13 ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Trung Nguyen
2026-07-01 17:13   ` [PATCH 2/2] selftests/hid: multitouch: test a large ContactCountMaximum Trung Nguyen
2026-07-01 19:12   ` [PATCH 1/2] HID: multitouch: fix out-of-bounds bit access on mt_io_flags Benjamin Tissoires

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox