[PATCH v2] HID: letsketch: fix UAF on inrange_timer at driver unbind

[PATCH v2] HID: letsketch: fix UAF on inrange_timer at driver unbind

[Manish Khadka]

letsketch_driver does not provide a .remove callback, but
letsketch_probe() arms a per-device timer:

    timer_setup(&data->inrange_timer, letsketch_inrange_timeout, 0);

The timer is re-armed from letsketch_raw_event() with a 100 ms
timeout on every pen-in-range report, and its callback dereferences
data->input_tablet to deliver a synthetic BTN_TOOL_PEN release.

letsketch_data is allocated with devm_kzalloc(), and its input_dev
fields are devm-allocated via letsketch_setup_input_tablet().  On
device unbind (USB unplug or rmmod), the HID core runs its default
teardown and devm cleanup frees both letsketch_data and the input
devices.  Because no .remove callback exists, nothing drains the
timer first: if raw_event armed it within ~100 ms of the unbind,
the pending timer fires on freed memory.  This is a UAF read of
data and of data->input_tablet, followed by input_report_key() /
input_sync() into the freed input_dev.

The same problem can occur on the probe error path: if
hid_hw_start() enabled I/O on an always-poll-quirk device and then
failed, raw_event may have armed the timer before devm releases
data.

Fix by adding a .remove callback that calls hid_hw_stop() first.
hid_hw_stop() synchronously kills the URBs that deliver raw_event(),
so once it returns no path can re-arm the timer.  timer_shutdown_sync()
then drains any in-flight callback and permanently disables further
mod_timer() calls.  Apply the same timer_shutdown_sync() in the probe
error path so the timer is guaranteed not to outlive data.

Fixes: 33a5c2793451 ("HID: Add new Letsketch tablet driver")
Cc: stable@vger.kernel.org
Signed-off-by: Manish Khadka <maskmemanish@gmail.com>
---

v1 -> v2:
  - Drop the spinlock_t and bool removing fields and their uses
    (and the <linux/cleanup.h> include): hid_hw_stop() synchronously
    kills URBs, so once it returns no raw_event can re-arm the timer
    and an explicit gate is unnecessary.
  - Use timer_shutdown_sync() in place of timer_delete_sync(), which
    also makes the probe error path safe against a stray mod_timer()
    that an always-poll-quirk hid_hw_start() failure could leave
    behind.
  - Drain the timer in the probe error path via a unified
    err_shutdown_timer label.
  Thanks to Sashiko AI review for both points.
 drivers/hid/hid-letsketch.c | 36 +++++++++++++++++++++++++++++++++---
 1 file changed, 33 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/hid-letsketch.c b/drivers/hid/hid-letsketch.c
index 11e21f988723..b52e93a91ae5 100644
--- a/drivers/hid/hid-letsketch.c
+++ b/drivers/hid/hid-letsketch.c
@@ -296,13 +296,42 @@ static int letsketch_probe(struct hid_device *hdev, const struct hid_device_id *
 
 	ret = letsketch_setup_input_tablet(data);
 	if (ret)
-		return ret;
+		goto err_shutdown_timer;
 
 	ret = letsketch_setup_input_tablet_pad(data);
 	if (ret)
-		return ret;
+		goto err_shutdown_timer;
+
+	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	if (ret)
+		goto err_shutdown_timer;
 
-	return hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+	return 0;
+
+err_shutdown_timer:
+	/*
+	 * Drain any pending callback and permanently disable the timer
+	 * before devm releases data: if hid_hw_start() enabled I/O on an
+	 * always-poll-quirk device and then failed, raw_event may have
+	 * armed the timer already.
+	 */
+	timer_shutdown_sync(&data->inrange_timer);
+	return ret;
+}
+
+static void letsketch_remove(struct hid_device *hdev)
+{
+	struct letsketch_data *data = hid_get_drvdata(hdev);
+
+	/*
+	 * hid_hw_stop() synchronously kills the URBs that deliver
+	 * raw_event(), so once it returns no path can re-arm
+	 * inrange_timer.  timer_shutdown_sync() then drains any
+	 * in-flight callback and permanently disables further
+	 * mod_timer() calls before devm releases data.
+	 */
+	hid_hw_stop(hdev);
+	timer_shutdown_sync(&data->inrange_timer);
 }
 
 static const struct hid_device_id letsketch_devices[] = {
@@ -315,6 +344,7 @@ static struct hid_driver letsketch_driver = {
 	.name = "letsketch",
 	.id_table = letsketch_devices,
 	.probe = letsketch_probe,
+	.remove = letsketch_remove,
 	.raw_event = letsketch_raw_event,
 };
 module_hid_driver(letsketch_driver);
-- 
2.43.0