Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero
@ 2026-07-30 12:43 Dave Carey
  2026-07-30 12:55 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Dave Carey @ 2026-07-30 12:43 UTC (permalink / raw)
  To: linux-input; +Cc: jikos, bentiss, 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


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

* Re: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero
  2026-07-30 12:43 [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero Dave Carey
@ 2026-07-30 12:55 ` sashiko-bot
  2026-08-25  9:32 ` Benjamin Tissoires
  2026-08-25 10:17 ` Benjamin Tissoires
  2 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-07-30 12:55 UTC (permalink / raw)
  To: Dave Carey; +Cc: linux-input, dmitry.torokhov

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

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

* Re: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero
  2026-07-30 12:43 [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero Dave Carey
  2026-07-30 12:55 ` sashiko-bot
@ 2026-08-25  9:32 ` Benjamin Tissoires
  2026-08-25 10:17 ` Benjamin Tissoires
  2 siblings, 0 replies; 4+ messages in thread
From: Benjamin Tissoires @ 2026-08-25  9:32 UTC (permalink / raw)
  To: Dave Carey; +Cc: linux-input, jikos

Hi Dave,

On Jul 30 2026, Dave Carey wrote:
> 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) {

Sorry, I couldn't get back to you with this. I had second thoughts on
this patch, and ended up asking claude whether we were having any
potential issues. Turns out that we are fine according to all code path
it could find to one tiny exception: if there is no HID_DG_CONTACTCOUNT
in the report, contact_count is initialized to -1 but will not get a
positive or null value. So semantically, the code is misleading, but
it's harmless because num_expected would still stay at the value 0.

On the other hand, if the device reports a signed contact count and a
negative value, the code with your patch would fix the negative value to
be 0, so that's an improvement actually.

Cheers,
Benjamin

>  		/*
> +		 * 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
> 
> 

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

* Re: [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero
  2026-07-30 12:43 [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero Dave Carey
  2026-07-30 12:55 ` sashiko-bot
  2026-08-25  9:32 ` Benjamin Tissoires
@ 2026-08-25 10:17 ` Benjamin Tissoires
  2 siblings, 0 replies; 4+ messages in thread
From: Benjamin Tissoires @ 2026-08-25 10:17 UTC (permalink / raw)
  To: linux-input, Dave Carey; +Cc: jikos

On Thu, 30 Jul 2026 08:43:36 -0400, Dave Carey wrote:
> 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.
> 
> [...]

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

[1/1] HID: multitouch: Fix stale MT slots when contact count drops to zero
      https://git.kernel.org/hid/hid/c/e8e60b6439ee

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


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

end of thread, other threads:[~2026-08-25 10:17 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-30 12:43 [PATCH v3] HID: multitouch: Fix stale MT slots when contact count drops to zero Dave Carey
2026-07-30 12:55 ` sashiko-bot
2026-08-25  9:32 ` Benjamin Tissoires
2026-08-25 10:17 ` Benjamin Tissoires

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