Re: [PATCH] HID: multitouch: stop the release timer from being rearmed on remove

[Benjamin Tissoires <bentiss@kernel.org>]

On Jul 23 2026, Aldo Ariel Panzardo wrote:
> mt_remove() quiesces the sticky-finger timer before stopping the
> hardware:
> 
>     timer_delete_sync(&td->release_timer);
> 
>     sysfs_remove_group(&hdev->dev.kobj, &mt_attribute_group);
>     hid_hw_stop(hdev);
> 
> timer_delete_sync() waits for a running callback and dequeues the timer,
> but it does not stop the timer from being armed again. The transport is
> still delivering reports at that point, and the report path rearms it:
> 
>     if (app->quirks & MT_QUIRK_STICKY_FINGERS) {
>         if (td->mt_io_flags & MT_IO_SLOTS_MASK)
>             mod_timer(&td->release_timer,
>                       jiffies + msecs_to_jiffies(100));
> 
> A report that arrives after timer_delete_sync() has returned therefore
> leaves the timer queued. td is allocated with devm_kzalloc() against
> hdev->dev, so it is freed when the driver is unbound, after mt_remove()
> returns. When the timer fires afterwards, mt_expired_timeout()
> dereferences the freed td:
> 
>     struct mt_device *td = timer_container_of(td, t, release_timer);
>     struct hid_device *hdev = td->hdev;
> 
>     if (test_and_set_bit_lock(MT_IO_FLAGS_RUNNING, &td->mt_io_flags))
> 
> Simply moving the teardown after hid_hw_stop() does not fix this on its
> own, because mt_expired_timeout() calls mt_release_contacts(), which
> walks hdev->inputs; the timer still has to be quiesced before
> hid_hw_stop() tears the input devices down.
> 
> Use timer_shutdown_sync() instead, which additionally makes any later
> mod_timer() a no-op, so neither ordering constraint has to be traded off
> against the other. This is the final-teardown pattern the function was
> introduced for, and hid-wiimote already uses it for the same reason.
> 
> Fixes: 4f4001bc76fd ("HID: multitouch: fix rare Win 8 cases when the touch up event gets missing")
> Cc: stable@vger.kernel.org
> Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260723224211.613112-1-you@example.com?part=1
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@gmail.com>
> ---
> Found by code inspection after Sashiko AI review flagged the teardown
> ordering while reviewing an unrelated patch of mine. I have not
> reproduced the use-after-free at runtime: it needs a report to land in
> the window between timer_delete_sync() returning and the device being
> unbound, which I have no way to drive reliably on the hardware I have.
> The window and the rearm path are visible in the code, and the fix does
> not depend on the race being hit.
> 
>  drivers/hid/hid-multitouch.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 0495152091e3..f25065b9ec66 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -2233,7 +2233,7 @@ static void mt_remove(struct hid_device *hdev)
>  {
>  	struct mt_device *td = hid_get_drvdata(hdev);
>  
> -	timer_delete_sync(&td->release_timer);
> +	timer_shutdown_sync(&td->release_timer);

Reviewed-by: Benjamin Tissoires <bentiss@kernel.org>

Cheers,
Benjamin

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