[PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero

[PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero

[Dave Carey]

The INGENIC 17EF:6161 touchscreen (Lenovo Yoga Book 9 14IAH10) reports
HID_DG_CONTACTCOUNT=0 in the frame immediately following the last finger
lift rather than omitting the frame entirely.  In mt_touch_report() the
existing code only updates num_expected when contact_count is non-zero,
so a zero contact count on the first packet of a new frame leaves
num_expected at its previous value (e.g. 2 for a two-finger gesture).
The sync check "num_received >= num_expected" then evaluates "0 >= 2"
and never fires, preventing INPUT_MT_DROP_UNUSED from releasing the
stale slots.  Those slots remain active in the kernel MT layer until the
next touch, at which point they are released in a batch alongside the
new contact — causing the userspace event consumer to miss the intervening
finger-up sequence and corrupt its gesture session state.

Fix by resetting num_expected to 0 when contact_count is zero and
num_received is still 0 (i.e., this is the first and only packet of the
frame, not a continuation packet in a multi-packet sequence).  With
num_expected=0 the sync check "0 >= 0" fires immediately, calling
input_mt_sync_frame() which drops the stale slots via INPUT_MT_DROP_UNUSED.

The num_received==0 guard is critical: continuation packets in a
multi-packet frame arrive after at least one contact has already been
processed (num_received>0), so they are correctly excluded from this
path and the existing multi-packet logic is unaffected.

Signed-off-by: Dave Carey <carvsdriver@gmail.com>
Tested-by: Dave Carey <carvsdriver@gmail.com>
---
v3:
- Resend as standalone patch; v2 was sent with incorrect subject "2/5"
  (as a ping reply to the original series) so it was not recognized as
  a versioned respin by the maintainers.  No code changes from v2.

v2:
- Restructured contact_count block per Benjamin Tissoires' v1 review:
  replace three-branch if/else-if/else-if with a cleaner two-branch
  form, dropping the outer if (contact_count >= 0) wrapper.
- Add prev_scantime != scantime guard to the zero-contact sentinel case.

 drivers/hid/hid-multitouch.c | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index ec04dbafbd99..56a3f29d4938 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -1321,21 +1321,18 @@ static void mt_touch_report(struct hid_device *hid,
 	 * Includes multi-packet support where subsequent
 	 * packets are sent with zero contactcount.
 	 */
-	if (contact_count >= 0) {
+	if (contact_count > 0)
+		app->num_expected = contact_count;
+	else if (app->num_received == 0 && app->prev_scantime != scantime) {
 		/*
+		 * New multi-report frame:
+		 *
 		 * For Win8 PTPs the first packet (td->num_received == 0) may
 		 * have a contactcount of 0 if there only is a button event.
-		 * We double check that this is not a continuation packet
-		 * of a possible multi-packet frame be checking that the
-		 * timestamp has changed.
+		 *
+		 * Some other devices use a sentinel frame with 0 to release all contacts
 		 */
-		if ((app->quirks & MT_QUIRK_WIN8_PTP_BUTTONS) &&
-		    app->num_received == 0 &&
-		    app->prev_scantime != scantime)
-			app->num_expected = contact_count;
-		/* A non 0 contact count always indicates a first packet */
-		else if (contact_count)
-			app->num_expected = contact_count;
+		app->num_expected = 0;
 	}
 	app->prev_scantime = scantime;
 
-- 
2.55.0

Re: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Preexisting teardown race condition in mt_remove() where the sticky fingers release timer can be re-armed after deletion, leading to a Use-After-Free.
--

--- Patch [1]: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero ---
commit de5f919b17036295856b898ddbe48949c82b0650
Author: Dave Carey <carvsdriver@gmail.com>

HID: multitouch: Fix stale MT slots when contact count drops to zero

The INGENIC 17EF:6161 touchscreen reports a contact count of 0 in the frame
immediately following the last finger lift. This commit fixes an issue where
the driver left num_expected at its previous value, preventing stale slots
from being released. It resolves this by explicitly resetting num_expected to
0 for a new multi-report frame.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index edb37b4c867e4..418925b03691b 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -1321,21 +1321,18 @@ static void mt_touch_report(struct hid_device *hid,
>  	 * Includes multi-packet support where subsequent
>  	 * packets are sent with zero contactcount.
>  	 */
> -	if (contact_count >= 0) {
> +	if (contact_count > 0)
> +		app->num_expected = contact_count;

[ ... ]

> +		app->num_expected = 0;
>  	}
>  	app->prev_scantime = scantime;
>  

[Severity: High]
This is a pre-existing issue, but while looking at mt_touch_report(), I noticed
a potential race condition during device teardown that might lead to a
use-after-free.

In mt_remove(), the release timer is deleted before stopping the hardware:

drivers/hid/hid-multitouch.c:mt_remove() {
    ...
	timer_delete_sync(&td->release_timer);

	sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
	hid_hw_stop(hdev);
    ...
}

Since hid_hw_stop() hasn't been called yet, the hardware is still active and
can generate events. If an incoming HID report is processed concurrently by
mt_touch_report(), it can re-arm the timer:

drivers/hid/hid-multitouch.c:mt_touch_report() {
    ...
	if (app->quirks & MT_QUIRK_STICKY_FINGERS) {
		if (!bitmap_empty(td->active_slots, td->maxcontacts))
			mod_timer(&td->release_timer,
				  jiffies + msecs_to_jiffies(100));
    ...
}

After mt_remove() completes, the devm-managed mt_device structure (td) is freed.
If the re-armed timer expires 100ms later, could mt_expired_timeout() execute
on the freed td structure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730124336.637339-1-carvsdriver@gmail.com?part=1