HID: letsketch: fix UAF on inrange_timer at driver unbind

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>
Signed-off-by: Jiri Kosina <jkosina@suse.com>

diff --git a/drivers/hid/hid-letsketch.c b/drivers/hid/hid-letsketch.c
index 11e21f98872327ecb1d4870c5a60c521d95d92e8..b52e93a91ae54225dbcf52e5f39007feea4f8955 100644 (file)
--- 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);